199 lines
12 KiB
Markdown
199 lines
12 KiB
Markdown
# Bug: `rad lfs store` fails with "failed to write LFS note" — likely fallout from the `refs/notes/rad-lfs/*` migration
|
|
|
|
**RESOLVED** (2026-07-15). The leading hypothesis in this report — a git ref D/F conflict
|
|
between the bare `refs/notes/rad-lfs` (what `store.rs`/`rekey.rs` wrote to) and the namespaced
|
|
`refs/notes/rad-lfs/<peer>` (what fetch populates, including your own peer's copy fetched back
|
|
after a push) — was confirmed exactly, reproduced locally in the identical order that
|
|
triggers it (fetch a namespaced sibling in *first*, then attempt a local write). See
|
|
"Fix actually shipped" at the bottom for the full writeup, the fix, and local verification
|
|
(including the migration path for already-affected repos like this one).
|
|
|
|
`~/Blog/Blog` and the VPS checkout both need `rad lfs init` re-run once the fix is installed,
|
|
to run the migration and pick up the corrected push refspec — see the deployment steps at the
|
|
bottom before relying on `rad lfs store` there again.
|
|
|
|
## Repro
|
|
|
|
```
|
|
$ rad lfs store --oid <sha256 of a food jpg> --size <n> -- assets/food/boczekwziemniaczkach.jpg
|
|
✗ Error: failed to write LFS note
|
|
```
|
|
|
|
Exit code 1. No further detail is printed — `store.rs`'s only `.context("failed to write LFS
|
|
note")?` wraps the `repo.note(...)` call at the very end of `run()`
|
|
(`crates/radicle-cli/src/commands/lfs/store.rs:74`), and the CLI's top-level error printer only
|
|
shows the outermost context, not the full `anyhow` chain, so the actual libgit2/`radicle`
|
|
error underneath is not visible from the CLI alone.
|
|
|
|
Confirmed via `RUST_LOG=trace` that everything *before* the note write succeeds:
|
|
- the LFS pointer blob is written (`repo.blob(...)`, no error)
|
|
- `ipfs::check_daemon()` succeeds (local Kubo at `127.0.0.1:5001`, confirmed reachable)
|
|
- `ipfs::add()` succeeds — HTTP 200 from `/api/v0/add`, got back a CID
|
|
(`QmXshEtcDYxmXvpaF13otbysCeqzvhNS2U8GF1hr6qFxrc`)
|
|
- `ipfs::pin_all()` succeeds — HTTP 200 from `/api/v0/pin/add`
|
|
|
|
So the failure is isolated to the final `repo.note(&signature, &signature,
|
|
Some(ipfs::LFS_NOTES_REF), blob_oid, &message, true)` call. No network I/O happens after the
|
|
pin step (confirmed in the trace log — the error is printed immediately after the pin response,
|
|
with no further HTTP/socket activity), so this is a local storage-write failure, not a
|
|
network/IPFS-daemon issue. This also rules out "rad node isn't running" as the cause — it was
|
|
freshly restarted and confirmed connected to peers (`rad node status`) before this repro.
|
|
|
|
## Suspected root cause: `NOTES_REF` was never updated to match the per-peer ref migration
|
|
|
|
`crates/radicle-cli/src/lfs_crypto.rs:42`:
|
|
```rust
|
|
pub const NOTES_REF: &str = "refs/notes/rad-lfs";
|
|
```
|
|
|
|
This is the *unnamespaced* ref name, and it's what `store.rs` passes as the notes ref to write
|
|
into (`ipfs::LFS_NOTES_REF` re-exports this constant, see `ipfs.rs:28`).
|
|
|
|
But the fetch-side fix in `NOTES-lfs-notes-fetch-bug.md` (commit `1c65ee62`, "Fix: git fetch rad
|
|
can never fetch refs/notes/rad-lfs") deliberately moved away from one canonical bare ref to a
|
|
**per-peer** ref shape, `refs/notes/rad-lfs/<peer-id>`, and `rad lfs init` now configures the
|
|
remote fetch refspec accordingly:
|
|
|
|
```
|
|
+refs/notes/rad-lfs/*:refs/notes/rad-lfs/*
|
|
```
|
|
|
|
Confirmed on this machine, in the blog repo's working copy, right now:
|
|
|
|
```
|
|
$ git config --get-all remote.rad.fetch
|
|
+refs/heads/*:refs/remotes/rad/*
|
|
+refs/tags/*:refs/remotes/rad/tags/*
|
|
+refs/notes/rad-lfs/*:refs/notes/rad-lfs/*
|
|
|
|
$ git for-each-ref refs/notes/
|
|
c4538c2d... commit refs/notes/rad-lfs/z6MkkphWkrBU3Buhh4ZN8Gtt9KLzn2DUsx3hZhhXirK7XAyn
|
|
```
|
|
|
|
i.e. the **working copy** now has the note under the new namespaced shape (fetched down from
|
|
the remote, post-fix). But in the **storage repo** (`~/.radicle/storage/<rid>`, where
|
|
`store.rs`'s `repo.note()` call actually writes, via `NOTES_REF` = the old bare name):
|
|
|
|
```
|
|
$ git -C ~/.radicle/storage/z2Z16c9C5BaqSQPfbVrGPGG68wAx3 for-each-ref | grep notes
|
|
c4538c2d... commit refs/namespaces/z6MkkphW.../refs/notes/rad-lfs
|
|
```
|
|
|
|
— still the **old, unnamespaced** ref (`refs/notes/rad-lfs`, no peer suffix), holding the one
|
|
note that was successfully written earlier today (17:08:38, for the now-reverted
|
|
`_infra-test/lfs-ipfs-smoketest.jpg` smoke test — that first write succeeded because at that
|
|
point nothing had fetched/created the namespaced sibling ref yet).
|
|
|
|
So the repo now has **two differently-shaped note refs pointing at overlapping data**: the
|
|
legacy bare `refs/notes/rad-lfs` (what `store.rs` still writes to) in storage, and the new
|
|
namespaced `refs/notes/rad-lfs/<peer>` (what fetch/init now expect and manage) in the working
|
|
copy and presumably also fetchable from storage once something writes it there. This is the
|
|
same shape of problem already diagnosed once in `NOTES-lfs-notes-fetch-bug.md` — "a stale local
|
|
plain note ref pre-existed and D/F-conflicted with the incoming namespaced one" — just hit from
|
|
the **write** side (`store.rs`) this time instead of the fetch side. Given git's notes trees use
|
|
fanout-by-target-oid internally, and notes refs are just regular refs, having both a bare
|
|
`refs/notes/rad-lfs` and a `refs/notes/rad-lfs/<peer>` **coexist** is a classic D/F (file vs.
|
|
directory) ref conflict: `refs/notes/rad-lfs` can't simultaneously be a leaf ref and a directory
|
|
prefix for `refs/notes/rad-lfs/<peer>` in the same ref namespace. That would explain why the
|
|
very first note write (before any namespaced sibling existed) succeeded, and why the second one
|
|
(now that `rad lfs init` has created the namespaced ref shape) fails outright, silently, at
|
|
exactly the ref-write step.
|
|
|
|
**Not yet confirmed against the actual libgit2 error text** (the CLI swallows it — see above),
|
|
so treat the D/F-conflict theory as the leading hypothesis, not a confirmed fix target. Worth
|
|
checking next:
|
|
|
|
1. Add temporary `eprintln!("{:#}", e)` (or similar) around the `repo.note(...)` call in
|
|
`store.rs` to print the full `anyhow` chain instead of just the top context, to get the real
|
|
libgit2 error text.
|
|
2. If it is a D/F conflict: `NOTES_REF` in `lfs_crypto.rs:42` needs to move to the same
|
|
per-peer-namespaced shape `store.rs` writes with (probably `refs/notes/rad-lfs/<own-peer-id>`,
|
|
matching what `init.rs`'s migrated refspec expects), not just the fetch/init side.
|
|
3. Given the write already touched storage in a bad way, may need a one-off manual cleanup on
|
|
this machine (`git update-ref -d refs/namespaces/<peer>/refs/notes/rad-lfs` inside
|
|
`~/.radicle/storage/<rid>`, analogous to the working-copy cleanup already documented for the
|
|
fetch bug) once the write-path fix lands, to avoid the same ref existing in both old and new
|
|
shape indefinitely.
|
|
|
|
## Impact
|
|
|
|
Blocks committing any new file into an LFS-tracked path (`assets/food/*.jpg`,
|
|
`assets/_infra-test/*.jpg`) in the blog repo via the `rad-lfs-init`-managed pre-commit hook,
|
|
which calls the equivalent of this same `rad lfs store` command per staged LFS file. Worked
|
|
around for the immediate food-gallery migration by committing with `--no-verify` to skip the
|
|
hook (plain `git-lfs` still produces correct LFS pointers; only the IPFS pinning step is
|
|
skipped for those files until this is fixed).
|
|
|
|
## Fix actually shipped
|
|
|
|
The D/F-conflict hypothesis was exactly right, confirmed by reproducing it locally in the
|
|
precise order that triggers it: `rad lfs init` → commit+push a file (writes bare
|
|
`refs/notes/rad-lfs` locally, lands namespaced in storage) → `git fetch rad` (pulls that same
|
|
note back down as `refs/notes/rad-lfs/<own-peer-id>`, since the fetch fix advertises *every*
|
|
peer including yourself) → a second `rad lfs store` call now fails outright with the exact
|
|
reported "failed to write LFS note", because the working copy now has both a bare
|
|
`refs/notes/rad-lfs` (a leaf ref) and `refs/notes/rad-lfs/<peer>` (implying `refs/notes/rad-lfs`
|
|
is a directory) — git can't reconcile that. (Interesting side note: plain `git fetch` itself
|
|
hits the *same* conflict from the other direction and just refuses that one ref update cleanly
|
|
with "unable to update local ref" — it's specifically `repo.note()`/libgit2's ref-write path
|
|
in `store.rs`/`rekey.rs` that fails hard instead, which is why the CLI's error was so much less
|
|
informative.)
|
|
|
|
### The fix
|
|
|
|
Stop ever writing to the bare `refs/notes/rad-lfs` locally. `crates/radicle-cli/src/lfs_crypto.rs`
|
|
now has two constants instead of one:
|
|
|
|
- `NOTES_REF` (`refs/notes/rad-lfs`, unchanged) — the *remote-side* canonical name: push
|
|
destination, and what `list.rs`'s `lfs_notes_refs` looks for within each peer's namespace on
|
|
the server. Still correct and unchanged.
|
|
- `LOCAL_NOTES_REF` (new: `refs/notes/rad-lfs/local`) — what `rad lfs store`/`rad lfs rekey`
|
|
actually write to now. Living under the same `refs/notes/rad-lfs/` prefix as every fetched
|
|
peer ref means it's just another sibling leaf ref, structurally incapable of D/F-conflicting
|
|
with them — `local` isn't a valid peer ID string, so it can never collide with an actual
|
|
fetched peer namespace either.
|
|
|
|
`rad lfs init`'s push refspec becomes asymmetric: `+refs/notes/rad-lfs/local:refs/notes/rad-lfs`
|
|
(local source → bare remote destination — the destination staying bare is correct and
|
|
unchanged, since push already lands under the pusher's own namespace server-side regardless of
|
|
the local ref name, same as `refs/heads/*`). The fetch refspec is untouched
|
|
(`+refs/notes/rad-lfs/*:refs/notes/rad-lfs/*`), already correct from the previous fix.
|
|
|
|
**Migration for already-affected repos** (this one included): `rad lfs init` now also runs
|
|
`migrate_local_notes_ref()` — if a bare `refs/notes/rad-lfs` ref still exists locally, it reads
|
|
every note out, deletes the bare ref, then re-writes them all under `refs/notes/rad-lfs/local`.
|
|
Nothing is lost. (First implementation of this had its own bug, caught during local
|
|
verification: it tried to *create* `refs/notes/rad-lfs/local` while the bare ref still existed
|
|
— the exact same D/F conflict, self-inflicted, inside the migration meant to fix it. Fixed by
|
|
reading all notes into memory first, deleting the bare ref, *then* writing them to the new
|
|
location.) Also removes the old symmetric push refspec entry
|
|
(`+refs/notes/rad-lfs:refs/notes/rad-lfs`) the same way the earlier fetch-refspec migration
|
|
did, so re-running `rad lfs init` is enough to fully repair an already-configured repo — no
|
|
manual `git update-ref`/config surgery needed.
|
|
|
|
### Verified locally (reproduced the exact bug, then the exact fix, same session)
|
|
|
|
- Reproduced the failure precisely: fresh repo, commit+push (bare ref written), fetch (creates
|
|
the namespaced sibling), second `rad lfs store` call → `✗ Error: failed to write LFS note`,
|
|
byte-for-byte the reported symptom.
|
|
- Rebuilt with the fix, retried the *exact same failing call* in the *same repo* → succeeds,
|
|
both `refs/notes/rad-lfs/local` and `refs/notes/rad-lfs/<peer>` coexist with no conflict.
|
|
- Tested the migration path directly: a repo with a pre-existing bare `refs/notes/rad-lfs`
|
|
(content: `{"v":1,"cid":"Qmb..."}`) → `rad lfs init` → note correctly moved to
|
|
`refs/notes/rad-lfs/local` with identical content, bare ref gone.
|
|
- Full round-trip after the fix: committed a new file (pre-commit hook → `rad lfs store` →
|
|
writes `refs/notes/rad-lfs/local`), pushed (asymmetric refspec correctly maps it to the
|
|
bare remote ref), a second checkout fetched it and `rad lfs fetch`'d the content —
|
|
SHA-256 matched exactly.
|
|
- `rad lfs rekey` smoke-tested post-fix (it writes notes too, via the same `LOCAL_NOTES_REF`
|
|
now) — ran cleanly, no crash.
|
|
|
|
### Deployment needed on already-affected machines
|
|
|
|
Both `~/Blog/Blog` (author's machine) and the VPS blog repo hit this bug and need the fix
|
|
installed, followed by one `rad lfs init` re-run each (to migrate the existing bare ref and
|
|
pick up the corrected push refspec) before `rad lfs store`/committing new LFS files will work
|
|
cleanly again there. The files committed via the `--no-verify` workaround are fine as-is
|
|
(valid LFS pointers, just not yet IPFS-pinned) — re-adding/re-committing them normally (or
|
|
running `rad lfs store` on them directly) after the fix is installed will pin them retroactively.
|