fix(modelartifacts): treat CIFS EACCES as lock contention, not failure

[localai-bot]

Problem

On a CIFS/SMB-backed /models, two controller replicas starting together both fail to materialize the same model and both silently fall back to legacy loading — which makes the worker download the entire repo in-band inside LoadModel and exceed the remote-load deadline.

flock() on CIFS returns EACCES, not EWOULDBLOCK, when another client holds the lock: the kernel maps STATUS_LOCK_NOT_GRANTED and STATUS_FILE_LOCK_CONFLICT to -EACCES, and there is no EWOULDBLOCK translation anywhere in the cifs lock path. gofrs/flock only recognises EWOULDBLOCK as contention, so TryLockContext returned a hard error and Ensure aborted.

The permission denied in the logs was the lock working correctly and being misread as a failure. Root and 0777 were irrelevant — no permission check ever ran.

Confirmed experimentally on the affected cluster, two pods against the shared mount:

pod A: flock -x /models/.artifacts/.locks/probe.lock -c 'sleep 90'
pod B: flock -n -x /models/.artifacts/.locks/probe.lock -c true
       -> flock: /models/.artifacts/.locks/probe.lock: Permission denied
       -> exit=65

flock(1) exits 1 silently on EWOULDBLOCK and only prints for other errnos, so this proves EACCES.

Fixes #10981.

Approach: disambiguate EACCES rather than replace flock

EACCES from flock on SMB is genuinely ambiguous at the syscall boundary, so treating it as contention naively would risk retrying forever on a real permission error. It is not ambiguous at this call site:

• gofrs/flock opens the lock file O_CREATE|O_RDWR (flock.go:184) before calling unix.Flock. A real permission problem therefore fails the open and returns an *fs.PathError naming the path. The production error was a bare, path-less permission denied — the diagnostic fingerprint that identified this bug in the first place.
• flock(2) on Linux documents no EACCES at all (EBADF, EINTR, EINVAL, ENOLCK, EWOULDBLOCK).

So a bare EACCES from the lock call, on an fd already held read-write, can only come from a network filesystem's lock-conflict translation. And the wait is bounded regardless, so a misclassification degrades to a delay rather than a hang — that asymmetry is what makes the cheap option defensible rather than merely cheap.

The O_CREAT|O_EXCL sentinel alternative, and why it was rejected

EEXIST survives SMB translation unambiguously, which is genuinely attractive. But a crashed holder leaves a stale sentinel, and the grace period would have to exceed the longest legitimate materialization — hours, for the 57 GB repo in this issue. Too short silently permits two replicas materializing concurrently, which is the same outcome as the nobrl booby trap. Too long blocks every replica for hours after a crash. Doing it correctly needs an mtime heartbeat refreshing the sentinel across replicas — replacing crash-safety the kernel gives away for free with crash-safety we would have to maintain.

The Postgres advisory lock (the "architecturally correct" option in the issue) was not pursued: it only works where Postgres is present, so the flock stays as the single-node path either way, leaving two mechanisms to keep in sync. The new Locker interface is the seam it would plug into later.

Changes

• acquireLock: explicit bounded wait loop replacing TryLockContext, treating EWOULDBLOCK/EAGAIN/EACCES/EBUSY as contention. Logs when it starts and stops waiting.
• ErrLockContended, returned only when the wait window expires. Ensure re-checks the committed result first, so a peer that finished the work wins and contention never reaches the fallback — losing the race is a success signal, not a failure.
• Locker interface + WithLocker / WithLockWait. Exported rather than test-only: it is both the seam that makes the contention path testable without a network filesystem, and the plug point a database-backed lock would use.
• config.LogArtifactFallback, shared by bindPrimaryArtifact and preloadOne. Logs at error for a managedArtifactBackends backend and names the in-band download, since for those backends "legacy loading" is a guaranteed slow path rather than graceful degradation. Still non-fatal. The gate is always true today, but is checked explicitly so widening PrimaryArtifactSpec later cannot silently promote an ordinary fallback to an error.
• Removed the redundant os.Chmod(layout.Lock, 0o600) — flock.New already creates the file 0600 with O_CREATE, so the chmod was a syscall that could only fail on a nounix mount with file_mode=0777 forced.

Testing

CIFS cannot be mounted in CI, so the errno is injected rather than the filesystem. The seam landed first as a faithful port of the old semantics, so the new specs failed on the real EACCES rather than on a missing symbol:

Summarizing 3 Failures:
  [FAIL] artifact lock contention ... waits out CIFS-style EACCES contention and then materializes
     [FAILED] Unexpected error: <syscall.Errno>: permission denied 0xd
  [FAIL] ... adopts the peer's snapshot when contention outlasts the wait window
  [FAIL] ... reports contention distinctly when the peer never commits
     Expected <syscall.Errno>: permission denied 0xd to match error
              artifact lock is held by another process
Ran 37 of 37 Specs -- 34 Passed | 3 Failed

The EWOULDBLOCK and ENOLCK specs passed throughout — that is the control, proving the seam preserved behaviour and only the classification changed. Severity specs capture at slog.LevelError so a warn emission is filtered out and the test fails on severity rather than wording.

go build ./core/... ./pkg/...   -> OK
go test ./pkg/modelartifacts/ ./core/gallery/ ./core/config/  -> all ok
make lint                        -> 0 issues

Grepped the repo including tests/e2e/ for the touched contracts — no other pins.

Notes for review

• preloadOne runs sequentially per model, so DefaultLockWait (30m) can stall startup behind a peer's large download. Waiting is still strictly cheaper than the fallback, which is guaranteed-slow and duplicates the same download — but this is a behavioural change worth a second opinion. The alternative would be deferring materialization on contention so the losing replica finishes startup and serves traffic sooner.
• WithLockWait is not yet wired to a CLI flag or env var; exposing it would touch the runtime-settings registry, so it is left as a follow-up.
• os.Rename(layout.Partial, layout.Final) still has no SMB EBUSY fallback. It is correct while the lock holds, and this PR restores that precondition on CIFS rather than weakening it — previously, contention silently sent both replicas down a path where the lock's guarantee no longer held.

---

🤖 Generated with Claude Code

[localai-bot]

Correcting the rationale in the description above, because it states the right conclusion for the wrong reason and would invite the same challenge from the next reviewer.

The description says the Postgres advisory lock was not used because "it only works where Postgres is present, so the flock stays as the single-node path either way, leaving two mechanisms to keep in sync."

That is wrong on the facts. TryWithLockCtx (core/services/advisorylock/advisorylock.go:77) already handles the non-Postgres case itself — isPostgres(db) false falls back to localLockChan(key). The package is self-contained; a caller does not maintain two mechanisms. It also has KeyFromString (:121) for dynamic per-artifact keys, and KeyGalleryDedup is precedent for the same problem class ("don't let two replicas do the same work"). Asking why materialization does not use it is a reasonable question.

The real reasons are lock ownership and duration.

Ownership. Postgres advisory locks have no fencing token. If the holder's connection drops, the lock is released silently and the original holder is not notified — it keeps writing to .partial/<cacheKey> while a second replica acquires the lock and begins writing to the same path. Both writers open with O_APPEND (pkg/downloader/uri.go:706) and the resume probe reads the other's in-flight size. The failure mode is therefore not merely duplicate work but interleaved bytes in a shared partial directory, caught by SHA verification only after both replicas have burned the entire download.

flock has no such gap: the kernel ties the lock to the process that owns the files. It is released exactly when that process dies, because it is that process's lock. An advisory lock is held by a connection with no causal relationship to the download — the connection can drop while the download continues, and the download can die while the connection lingers. Any liveness protocol added on top is an attempt to re-establish a coupling flock gets for free.

Duration. Every existing advisory-lock use in this tree is a short critical section or a ticker-guarded leader election; RunLeaderLoop (leader_loop.go:16) skips the tick on contention rather than waiting. None holds across unbounded I/O. A multi-hour hold on a pooled connection would be the first of its kind, and a bug fix is not where to pioneer that.

Wiring, independently. authDB is initialised at core/application/startup.go:139, but the materializer is constructed earlier (core/cli/run.go:248) and passed in via config.WithModelArtifactMaterializer. Of the five construction sites, only the startup path has a DB in scope at all, and there it arrives after the object exists. core/cli/models.go:87 (models install) has no DB in any deployment.

Known limitation this PR does not address

.partial/<cacheKey> is shared mutable state with no second line of defence — it is safe only because the lock holds. This PR restores that precondition on CIFS rather than removing the dependency.

A follow-up worth considering, deliberately not done here: make the partial path writer-unique (.partial/<cacheKey>.<writerID>), so concurrent writers cannot corrupt each other regardless of lock behaviour, and the lock becomes a pure efficiency optimisation rather than a correctness dependency. That would also open the door to a brief advisory lock electing a writer plus an expiring on-disk claim — a short critical section, consistent with existing usage — without holding anything across the download. Worth driving from an observed corruption or repeated duplicate downloads, not from this incident, which this fix resolves.