grenadeandClaude Opus 5
3790f2589c
The lock added alongside compare-and-set removed its map entry inside the critical section. A queued waiter still held that mutex, so the next arrival found no entry, created a fresh one, and the two then ran concurrently -- defeating the lock exactly when there was contention, which is the only time it does anything. I previously reported this fix as working on the strength of ten clean release-mode runs. That was wrong: in debug, where the timing differs, the guarding test fails 3 times in 12. Retiring the entry only after dropping our own reference, under the same shard lock `entry` takes, gives 0 failures in 25 -- and restoring the old behaviour reproduces 3 in 12, so the test really does catch it. The consequence was the one the test names: a file served to a client but never recorded, and so invisible the moment the instance is sealed. Real against the local filesystem store, which has no conditional put and relies on the verify-after-write retry; MinIO's compare-and-set covers the same race, which is why the deployed path never showed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XZG2i4AmfSqE97EJGBVb64
4.6 KiB
rustingface — notes for agents
Read doc/spec.md first; it is the contract, not a sketch. Then
read ~/git/architecture/generic.md for the conventions this repo inherits
(workspace layout, deployment, systemd, firewalld, SELinux, commits) and
port-allocations.md for why the port is 20482.
What must not regress
The five guarantees in doc/spec.md §1 are contractual. In particular:
- Never normalise a replayed header.
ETag,X-Repo-Commit,X-Linked-EtagandX-Linked-Sizeare recorded as upstream sent them and emitted unchanged, quotes included. Client cache layouts are keyed on these. Do not compute a git blob SHA-1, derive an etag from content, or decide what a commit "should" be. - Never let a redirect eat the Hub's headers. The Hub answers a resolve
request with a redirect and puts the replayable metadata on that response;
the CDN it points at has none of it.
rustingface-data's upstream client follows redirects by hand for this reason. If you ever re-enable reqwest's own redirect policy, every recorded etag silently becomes wrong. - Never set
Content-Encodingon the resolve path. The client sendsAccept-Encoding: identityprecisely so the sizes it learns are real. - Never write a manifest entry before its blob upload has completed. The
ordering in
rustingface-core/src/fetch.rsis deliberate: a crash may leak an orphan blob, whichgcreclaims, but a manifest entry pointing at incomplete bytes is a silent correctness failure. - Never complete a client response before the manifest write lands. The tee runs one chunk behind for this reason. A file that was served but not recorded is invisible the moment the instance is sealed.
Testing
cargo test --workspace covers it, including tests/sovereignty.rs, which
runs the real service against a fake Hub speaking the real header protocol.
Add a case there for any behaviour a client depends on.
The conformance suite (test/conformance/) needs network and a pinned
huggingface_hub. Run it before claiming a change is safe for clients — most
of the real bugs in this repo's history were found by it and not by the unit
tests. It has also been run against the live MinIO on caveman; do that for
anything touching rustingface-data, because the local filesystem store hides
whole classes of problem (it has no multipart, no conditional update, and no
presigning).
The web UI
web/ is a Vite + React + SWC + TS app (architecture/generic.md §4), served
statically from the nginx host and reading the /v1/ API.
- Keep UI routes out of
/api/. That surface is the Hub's, replayed verbatim. Anything rustingface invents goes under/v1/. - Model cards are untrusted third-party content.
components/Markdown.tsxparses their HTML and then sanitises it; the plugin order (rehype-raw, thenrehype-sanitize) is not interchangeable, and reversing it would reintroduce the raw HTML unchecked. Card images are deliberately not loaded. - Any new route must not shadow a repository id. rustingface owns the whole
URL root, which is the Hub's namespace, so every prefix collides with some
repo. Where it is ambiguous, disambiguate in the handler the way
v1_repo_detaildoes — and test it against/resolve/in a directory name, not just in a repo name.
Auth, and what actually bounds the bucket
Two credentials that are easy to confuse, pointing in opposite directions:
upstream.token_file(hf-token) authenticates rustingface to the Hub. Outbound only, never echoed downstream, never logged. Needed for gated repos.auth.token_file(client-tokens) authenticates clients to rustingface.
A client's token is never forwarded upstream, and rustingface's token is never shown to a client. Do not let these merge.
auth.mode = "bearer" controls who, not what — a token holder can still
cause any repository to be fetched. policy.allow_new_repos = false is what
bounds the bucket, and the deployed configuration relies on it.
auth.anonymous = "catalog" opens /v1/ to unauthenticated requests. That is
only safe while every handler under /v1/ reads the bucket and never consults
upstream. If you add a route there that can fetch, you have made the public
face able to fill the bucket — there is a test asserting the whole surface
causes zero upstream requests, and it should stay that way.
Where things live
Types in entities, logic in core, I/O in data, HTTP in api, wiring in
the binary. core defines the Store and Upstream ports and must not gain a
direct dependency on object_store or reqwest.