A worker whose link is silently broken reconnects to another replica while
the old owner's socket has not yet noticed. Without a fence both believe
they own it. Claim is a single atomic upsert returning the new epoch, and a
release must match both owner and epoch so a stale owner cannot delete a
live claim.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The peer link's WebSocket adapter and route constant lived in
core/http/endpoints/cluster, so the dialler in core/services/cluster had to
import an HTTP endpoints package to reach them. That pulled echo, core/http/auth
and core/config into a package whose doc says it is deliberately free of such
dependencies, and it made core/services/nodes reach an endpoints package
transitively. It also has no way forward: the worker-connect handler needs the
tunnel registry and the node token store, both of which are cycles from there.
Move WebsocketConn and PeerPath into core/services/cluster and let the endpoints
package import it, which is the direction the rest of core/http flows. The route
and the auth exemption still cannot drift apart, now asserted where both are
visible rather than by a const reference across the boundary, and the assertion
is stronger than the one it replaces: it pins the route under the prefix instead
of pinning the prefix's spelling.
Also guard the fresh-dial path with ctx.Err(), mirroring the cached path. A
caller with a 300ms deadline dialling a live, listening peer was told the peer
was unreachable, which would be enough for one impatient client to get a healthy
replica routed around once the relay consults these errors.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Distinguishes a peer missing from the registry from a peer that will not
answer: the second must never be readable as node absence, or a network
hiccup between replicas evicts healthy workers.
The distinction is a property of the error type rather than of the call
sites. The unreachable error formats its cause into its message and keeps
it out of its unwrap chain, so an ErrInstanceNotFound picked up on the
dial path cannot reach a caller's absence check.
One yamux session is cached per peer and re-dialled when OpenStream on it
fails, which is how both a dead transport and a graceful remote go-away
arrive. A reset of one stream never reaches the pool, so an abandoned
request cannot cost every other worker its link.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The prefix constant moves to core/http/auth beside the check that uses it, and
the endpoints package derives its route from there. Seven sibling endpoint
packages already import auth, so the previous direction would have deadlocked
the build as soon as this one registered in RouteFeatureRegistry, and it was
dragging echo, gorilla/websocket and yamux into unrelated service packages.
Four properties were argued in comments and held by nothing. Flipping the
empty-token check to fail open, making SetWriteDeadline a no-op, returning a
zero-length read for a zero-length message, and dropping the recover around the
callback all left the suite green. Each now fails a spec that asserts the
behaviour rather than the setter's return value.
SetWriteDeadline takes the write mutex because gorilla keeps that deadline in a
plain struct field applied at the next flush; SetReadDeadline must not take the
read mutex, since it goes straight to the net.Conn and would otherwise block
behind the read it exists to unblock.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A dead yamux session does not always arrive wrapped. Session.close publishes
shutdownErr and closes shutdownCh before it force-closes the streams, so a
Write or Close landing in that window gets the raw cause back instead
(session.go:507-510, 528-533), and for a peer that vanished the raw cause is a
bare io.EOF. The generic io.EOF clause then reported the dead session as a
clean completion.
Remove the clause. A clean read-side EOF never reached it anyway: io.Copy
consumes that and reports nil, and neither *yamux.Stream nor *net.TCPConn
takes a WriteTo/ReadFrom path that would hand one back. Every existing spec
still passes, the io.EOF entry in the normal-termination table included, which
is what showed the branch was dead for legitimate endings and live only for
the bug.
Add a spec driving a real yamux session end to end. Every mux shape until now
was a synthesized error, which is exactly why a race inside the real library
stayed invisible.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Upgrades to a WebSocket, wraps it as a yamux server session and hands it to
the caller. Rejects before upgrading so an unauthenticated dial sees a 401
rather than a WebSocket error, which is what the route-coverage test asserts.
The adapter keeps the reader of a partially consumed message across Read
calls. yamux reads through a 4 KiB bufio.Reader, so a small-payload test
cannot see a dropped message tail; the framing specs drive the adapter
directly with buffers smaller than the message.
An empty configured token authorizes nobody here, unlike the worker file
transfer server's check: this route is registered in every deployment, so
failing open would publish an unauthenticated mux.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Matching yamux errors with errors.Is was too broad. Session.close hands every
live stream ErrStreamReset wrapped around whatever killed the connection, so a
keepalive timeout, a broken TCP connection or a peer that simply vanished all
matched, and a relayed request that died reported a clean ending. Nothing
upstream would have retried or logged it.
Match the plain sentinels by identity, since only identity separates a stream
that was reset from the wrapped form that means the session died. Treat a
StreamError as a per-stream reset, and a GoAwayError as normal only when it
carries the no-error code, read off ErrRemoteGoAway because the constant is
unexported. ErrSessionShutdown needs no entry of its own; it is a GoAwayError
with that code.
Order matters as much as the matching: session death wraps its cause, which is
routinely io.EOF or a closed socket, so the mux checks run before the generic
endings. Reversing them alone puts a vanished peer back to nil.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
go-yamux/v5 matches none of its errors against net.ErrClosed, so the
classifier reported an ordinary teardown as a failure: when the session has
gone away, the FIN that Splice's own Close writes returns ErrSessionShutdown,
and a stream torn down under a live copy surfaces as ErrStreamClosed or a
reset. Splice owns that Close, so it owns the errors it produces; the
sentinels are named here rather than injected by the caller, which would make
a forgotten classifier reintroduce the same bug silently.
Cover the error half of the contract, which no in-memory pipe could reach: a
scripted stream now feeds Splice a genuine transport failure and each
closed-stream ending in turn. Replacing the tail of Splice with "return nil"
passed every previous spec.
Also assert that Splice does not return until the second direction has
finished, rename a spec that promised a leak check it never made, and correct
two comments that claimed more than the code did.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Returns on the first direction to finish and closes both sides so the other
unblocks; a sequential copy deadlocks on any protocol where the far side
speaks first. EOF and use-of-closed are normal termination, not errors.
The fourth spec covers a peer that stops reading mid-body, the case where a
copy is parked in Write rather than in Read. The other three tear down an
idle splice and pass even against a Splice that closes only one side.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
DiscoverAdvertisedAddr promised to return an error rather than a fallback
no peer can dial, but only rejected an unspecified address. With PostgreSQL
on the same host or pod as a replica, which is compose, single-node and any
sidecar layout, the route to it is loopback, so every replica advertised
127.0.0.1 and a peer dialling that reached itself. Loopback, link-local and
zoned source addresses are now rejected with an error naming the remedy, and
a port outside 1-65535 is rejected before it becomes an undialable address.
Liveness was also measured on each replica's own clock: Register and
Heartbeat stamped last_seen from the Go process, and Live compared those
rows against the reading replica's time.Now(). Skew therefore shrank or
stretched the window by writerBehind+readerAhead, evicting healthy peers or
keeping dead ones. Both sides now use the database clock, which is the one
clock every replica demonstrably shares.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Replicas need to find each other to relay worker traffic, and nothing in
the tree recorded a replica's address. The advertised address is discovered
by opening a UDP socket toward PostgreSQL and reading back the local
address, which yields the interface every replica demonstrably shares
without asking an operator to configure one.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The closure note in cluster/failure.go quoted a Gomega error that Gomega
does not emit. Describe the argument-count failure and the
Eventually().WithArguments() hint instead, so nobody greps for a string
that never appears.
The advisory-lock note in cluster_failover_test.go called the wedge
window unbounded. A SIGKILLed local child closes its socket at once, the
Postgres backend reads EOF and is reaped in milliseconds, so the
mechanism bounds the window tightly. Say bounded, and keep the low
probability but real framing, which was right.
The workflow comment attributed HealthCheckInterval to
core/services/nodes/health.go. It is declared in
core/config/distributed_config.go:64; health.go only carries the ticker
on the unexported checkInterval. Point a debugger at the right file.
Comments only, no behaviour change.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review of the whole branch found five comments that would send a reader to
the wrong place, plus three smaller inaccuracies. Nothing here changes
behaviour.
The KNOWN RACE note on both backend-log WebSocket handlers said the fix
needs an atomic snapshot-plus-subscribe "under the store lock". It does
not: BackendLogStore.mu guards only the buffers map, and AppendLine
enqueues and fans out under the per-buffer buf.mu. Whoever took the store
lock would ship and the race would survive, so both notes now name buf.mu
and say what s.mu does and does not exclude.
Two comments in the cluster harness quoted Eventually(c.FrontendAlive)
.Should(BeFalse()). FrontendAlive takes an index, so Gomega rejects that
with "requested 1 arguments but received 0". Both now quote the closure
form the specs actually use, and say why the closure is needed.
proveHealthCheckingIsAlive claimed to prove the health monitor ran for the
whole preceding window. It proves the monitor was alive at the end of it,
and inferring backwards needs any wedge to be sticky. In the
peer-replica-death spec that inverts: health checks are single-flighted by
a session-scoped pg_try_advisory_lock, the spec SIGKILLs the replica that
may hold it, and until Postgres reaps the session the survivor acquires
nothing and checks nothing silently. Consistently(healthy) can then pass
because nothing was checking, with the positive control still succeeding
once the lock frees. The doc now states what is proven, names that gap,
and says the assertion is a floor rather than a proof.
The Makefile still called DISTRIBUTED_TEST_FLAKES a retry count, which is
what seeded that error into the two docs just corrected against it, and
the workflow called the 15s window a reconcile tick when the mechanism is
HealthCheckInterval in the node health monitor.
Also: the cluster suite measured 509.1s / 509.8s / 512.3s, so about
8m30s and not the 8m39s/8m40s three files claimed; the dead-worker spec
title implied two independent detectors when both probes read one
advisory-lock-serialised verdict out of the same row; and the
sanitizeDBName length assertion used <= 50, which an empty string also
satisfies, where the invariant for an over-long input is exactly 50.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
--flake-attempts is total attempts, not retries: ginkgo v2.29.0 sets
maxAttempts = FlakeAttempts and loops attempt < maxAttempts, and the flag's
usage string reads "0 - failed tests are not retried". At 1 there is no retry
at all, so "retries a failing spec once" was false in CONTRIBUTING.md and
implied in .agents/building-and-testing.md. Both now say each spec runs once,
and cite the source so the next reader need not re-derive it.
Also restores the React-UI stub rationale, which is load-bearing because a spec
asserting on a UI asset passes locally against a real dist/ and is served the
stub in CI; explains why 213 and ~240 differ; records that the workflow also
triggers on master pushes, where paths-ignore does not apply; and completes the
LOCALAI_E2E_REQUIRE_BINARIES value table, including that any unparseable value
reads as ON.
In .agents/ci-caching.md the stale "13 of those 20" figure now carries its
qualifier inline rather than in the following sentence.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Two Make targets, a flake-budget variable and two environment variables
landed with no way to discover them. CONTRIBUTING.md now tells a contributor
how to run both suites, what each costs and which variables steer the cluster
one.
.agents/building-and-testing.md records the decisions that are easy to undo by
accident: suite-scoped containers, the shared NATS bus and what that means for
a new spec, BeforeSuite over SynchronizedBeforeSuite, the label split,
--fail-on-empty, the binary gate, the flake budget of 1, the coverage
exclusion, and why the cluster suite's long waits must not be shortened.
.agents/ci-caching.md lists tests-e2e-distributed.yml in its paths-ignore
inventory; the workflow already pointed readers there, so the cross-reference
was dangling.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Ginkgo exits 0 when a label filter matches nothing, so a refactor that
renamed or dropped Label("Cluster") would have left the job reporting
"Test Suite Passed" having started no cluster. LOCALAI_E2E_REQUIRE_BINARIES
does not cover that case: it only fires inside a spec that is already
running. Add --fail-on-empty to both distributed targets.
Drop -r from test-e2e-cluster while here. All six Cluster specs live in the
top-level package, and the cluster subpackage contributes nothing under this
filter by design, so recursing only widened the blast radius. test-e2e-
distributed keeps -r: it must reach the eight argument-validation specs in
that subpackage.
Raise the cluster job to 45 minutes, matching its sibling. The 20 minute
Ginkgo timeout bounds the suite alone; the job timeout must also cover setup,
which is the larger and more variable half here: cold-cache module download,
protoc and protogen-go, a full build of ./cmd/local-ai and a separate test
compile, realistically 8-12 minutes on a 4-vCPU runner. At 25 minutes the
runner would have hard-killed the job before Ginkgo could report which spec
hung, which is the red-with-no-evidence outcome that gets suites disabled.
Also move upload-artifact to @v7 with the rest of the repo, and note on the
react-ui stub step that it must go if a spec ever asserts on a UI asset,
since a developer box has a real dist/ and would not catch that locally.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Add test-e2e-cluster and a second CI job that runs it. The cluster specs
spawn local-ai as real child processes and kill them, so they need a built
binary; keeping them in their own job means the fast in-process suite is not
held behind that build.
The binary is built with a stubbed core/http/react-ui/dist. A single
index.html satisfies the go:embed in core/http/app.go, and this suite drives
the HTTP API only, so the job skips a Node and Vite install entirely.
The job runs serial and pins --flake-attempts 1. Each Ginkgo process would
otherwise get its own PostgreSQL and NATS container while every spec spawns
two or three children, and a retry would hide exactly the nondeterminism the
suite exists to catch. Measured at 8m39s over three runs, hence a 25 minute
job timeout and a 20 minute Ginkgo timeout.
LOCALAI_E2E_LOG_DIR points inside the workspace so the per-process logs
upload as an artifact on failure; they are the only way to read a cluster
failure. LOCALAI_E2E_REQUIRE_BINARIES is set explicitly even though CI
already implies it, because a skipped cluster spec is indistinguishable from
a passing one and this job's whole value is that it cannot go green without
starting a cluster.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The two specs that assert a healthy worker stays healthy were pure negatives:
they say nothing happened. A cluster whose health checking had wedged, by
leaking the advisory lock the monitor takes at health.go:110, would freeze the
roster and satisfy both while observing a corpse. Kill the worker once the
window closes and require the roster to settle it to offline, so the preceding
Consistently is a statement about behaviour rather than about a stopped clock.
Applied to the cold-restart spec as well as the peer-death one: a restart is
exactly the event that could leave a replacement unable to check anything.
Document the hazard that can make an offline assertion hang. The staleness
branch skips a node already marked unhealthy (health.go:153-155), a skip meant
for nodes an operator took down, which also swallows the flap: an unhealthy mark
landing after the heartbeat goes stale means MarkOffline is never called and the
node stays unhealthy forever. Name the file and line at the assertion, and have
the failure message say so when the roster shows a node stuck there, so a
timeout sends the reader to LocalAI rather than to the harness.
Stop calling the two-replica registration spec a race. Start spawns workers
sequentially and the registrations land about a second apart; it is a
shared-roster identity test, and saying otherwise invites someone to trust it
for something it does not check.
WorkerRegistrar now bound-checks its index like every other index-taking method
here. It answered 0 for an out-of-range worker, and 0 is a real frontend index,
so the failure mode was a spec killing the wrong replica.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Four scenarios with no prior equivalent: killing a replica must not disturb a
worker that never depended on it, a cold-restarted replica must rehydrate the
roster from shared state and keep accepting the worker's heartbeats, a dead
worker must settle to offline on every replica, and two replicas registering a
worker each must converge on one roster.
The timings are measured, not assumed. Node liveness is heartbeat freshness, so
the only eviction path is StaleNodeThreshold (60s) plus one HealthCheckInterval
tick (15s), and neither is reachable from the CLI. A worker whose registrar was
killed was observed going offline at 74.2s. Every window here is sized to
outlast that, because an assertion that expires before the system could have
reacted proves nothing.
Two assertions are deliberately unlike the obvious form. Statuses are compared
for equality against a probe that returns a sentinel on error, rather than
asserting a name is absent from the healthy list: the list probe returns nil on
any error, and "does not contain" is satisfied by nil, so a 401 at the second
replica would have passed while observing nothing. And a killed worker is
required to settle to exactly offline, because it first flaps to unhealthy at
~8s and back to healthy at ~14s, which any not-healthy matcher would accept.
SpreadWorkerRegistrations is new, off by default, and exists so the racing
spec is a race: the harness otherwise points every worker at frontend 0, which
would have left that scenario asserting on two sequential writes through one
process. The default is unchanged because the baseline specs depend on it.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The previous round made a missing binary fail instead of skip, but only when a
workflow remembered to set LOCALAI_E2E_REQUIRE_BINARIES. That leaves the silent
pass one forgotten line away: the Cluster label partition is two specs, Ginkgo
exits 0 on skips, and a job that skips both reports "0 Passed | 2 Skipped" and
goes green having never started a cluster.
So the polarity is inverted. Binaries are required whenever CI is set, which
GitHub Actions always does, and the flag now exists to force the requirement
OFF rather than to be remembered ON. A local developer sees no change, since CI
is unset in an ordinary shell and a missing binary still skips with a message
naming the path and how to build it. off, no, n and disabled are honoured as
off; ParseBool rejects them, and reading a word that unambiguous as its
opposite would be a worse trap than the one this removes.
Also correct a claim the previous commit message got wrong. Comparing the
worker's registration id across the two replicas does not pin the topology:
NodeRegistry.Register looks a node up by name and preserves the existing id,
and both replicas read one Postgres, so registering the worker with every
frontend would yield identical ids too. The assertion is still worth keeping
for what it does catch, a replica answering from its own registry or database
instead of the shared one, and the comment now says that and nothing more.
The topology fact moves to where someone would break it: a note on
LOCALAI_REGISTER_TO recording that workers register with frontend 0 only, that
the cross-replica specs depend on it, and that nothing in those specs can
detect a change to it.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The Cluster label partition is these two specs and nothing else, so a missing
binary skipped the entire job. Ginkgo exits 0 on skips, so a build step that
broke or moved its output would have left the job reporting "0 Passed |
2 Skipped" and going green without ever starting a cluster: the silent pass
this suite exists to make impossible. Skipping stays the local default, which
is the right courtesy for someone who has not run `make build`, but
LOCALAI_E2E_REQUIRE_BINARIES turns it into a failure that names the missing
path and the target that builds it. A value that is set but unparseable counts
as on, since reading it as off would restore the very skip it disables.
Failures also name themselves now. The roster poll kept returning a bare nil on
error, so a 401 at the second replica, a decode failure and "the worker never
registered" all presented identically as an empty list. It now retains the last
error and the last roster and reports whichever happened, through a lazily
evaluated Gomega description that costs nothing until something fails.
Finally, the two-frontend spec no longer depends on the harness to mean what it
says. It asserts an unauthenticated GET /api/nodes at frontend 1 is refused,
which observes the admin gate instead of assuming it, and it compares the
worker's registration id across the two replicas rather than its name. A future
harness that registered every worker with every frontend would have kept a
name-only assertion green while it quietly stopped proving anything about
shared state.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Tasks 4 to 6 built a harness that runs local-ai as real child processes, but
none of it had ever started a process: every spec so far returned inside
argument validation. These two specs are the first to run it against a real
binary, a real Postgres and a real NATS.
Two frontends against one database both see a worker that registered through
only one of them. Every failover spec assumes this, so it is asserted first.
One admin session is minted at frontend 0 and reused for both replicas rather
than registering per frontend. The auth routes share a five-per-minute-per-IP
limiter and all e2e traffic is 127.0.0.1, so a session per frontend would
exhaust the budget as soon as a spec needs a third one. Reuse is sound because
sessions live in the shared Postgres and the harness pins one HMAC secret
across replicas; frontend 1 answering /api/nodes with 200 on a cookie minted at
frontend 0 is what proves it.
The binaries are resolved before SetupInfra so a missing build skips without
first provisioning a database the skip would then have to tear down.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review round 1. Comments only, plus one guard.
The note on Process.alive claimed the exited check closed the zombie window.
It does not. The reaper closes exited only after Cmd.Wait returns, and Wait
marks the os.Process done before returning, so exited being closed implies
signal 0 already errors and the branch cannot fire earlier than the one it
precedes. The window between the child exiting and waitid collecting it stays
open in both versions, and the only real mitigation is for callers to poll
with Eventually rather than sample once. Keep the check as hygiene, say what
it actually does, and say it again on the exited field, so nobody reads the
old claim and drops the Eventually.
Record what the cold wipe destroys. The harness sets no LOCALAI_STORAGE_URL,
so the object store is a directory under DataPath, and quantization and
fine-tune outputs live there too. Postgres keeps the job row; the artifact it
points at does not survive the restart. A spec that asserts otherwise will
fail for a storage reason wearing a failover costume.
Tell callers to let a graceful stop finish before restarting: RestartFrontend
terminates with SIGKILL, so pairing it straight after StopFrontendGracefully
cuts the drain short and silently converts the rolling-update case into the
crash case.
Refuse to wipe when the cluster has no work dir. frontendDataDir is relative
when baseDir is empty, so a Cluster built by some future test helper without
one would have RemoveAll walking frontend-N/data inside the source tree. The
guard sits before terminate, so a refusal leaves the cluster as it was.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The point of running LocalAI as real child processes is to be able to take
one away. Add KillFrontend (SIGKILL, the lost replica), StopFrontendGracefully
(SIGTERM, the rolling update), KillWorker, RestartFrontend and FrontendAlive.
RestartFrontend pins the dead replica's original port. Workers read
LOCALAI_REGISTER_TO once at boot and never re-resolve it, so a replica that
returns on a fresh port is unreachable by exactly the workers that registered
with it and the failover under test never happens.
It also wipes the replica's data directory, so the process comes back with
empty local state and has to rehydrate node, session and job state from the
shared Postgres and NATS. Reusing the directory would model a pod with a
persistent volume and hide the class of bug these tests exist to find. That
is only safe because the harness pins LOCALAI_AUTH_HMAC_SECRET; otherwise the
wipe would take {DataPath}/.hmac_secret with it and every session minted
before the restart would 401 afterwards.
FrontendAlive consults the reaper's exited channel before signal 0: a child
that has died but has not yet been waited on is a zombie, and signal 0 to a
zombie succeeds, which would report a dead replica as alive.
The new specs cover argument validation only. Killing, stopping and
restarting a live process needs a built binary plus Postgres and NATS, so
those paths stay unexecuted until the failover suites land.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Session rows are keyed by an HMAC of the token under a secret generated
per instance into {DataPath}/.hmac_secret. The replicas shared that
secret only because they shared a working directory, and that directory
was the source tree. Give each frontend LOCALAI_DATA_PATH under its own
baseDir and pin LOCALAI_AUTH_HMAC_SECRET, so a session minted at one
replica resolves at every other one by construction.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The register handler answers 201 both for "user created, here is your
session" and for "this email already exists", so the status code cannot
tell a fresh registration from a repeat one. Key on the session cookie
instead and fall through to login when it is absent.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Restarting a frontend replica must not move it: workers read
LOCALAI_REGISTER_TO once at boot and never re-resolve it, so a replica that
returns on a fresh port is unreachable by the workers that registered with it.
startFrontend now takes the port, with <= 0 meaning "allocate".
Process logs are opened for append rather than truncated, so a restarted
process cannot erase the log of the instance that died, which is the log a
failover post-mortem needs. The post-SIGKILL wait is bounded, so one stuck
child no longer becomes a suite-wide timeout that names nothing. Stop is
nil-safe because Start returns a nil cluster after stopping itself.
Start's doc comment no longer claims to wait for worker registration; that
needs an authenticated admin session, so it now says callers must poll
/api/nodes themselves.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Runs local-ai as real child processes, one per frontend replica and one per
worker, against containerised infrastructure. The in-process suites cannot
express frontend-replica failure: there is no process to kill and no real HTTP
boundary between a worker and the frontend it registered with.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The path allowlist covered 13 of the 99 packages the suite reaches. Commit
1dc3aeef8 touched core/config, core/services/modeladmin and core/backend and
matched no entry, so it would have merged without running the very specs that
cover it. Use the paths-ignore denylist tests-e2e.yml already uses.
Disable the testcontainers reaper: the runner is ephemeral, so the reaper buys
nothing and its unpinned image was pulled mid-suite, defeating the pre-pull.
Drop continue-on-error, which no other workflow uses and which reports a failed
run as green. The job is advisory by staying out of branch protection instead.
Pin Go to 1.26.0 to match go.mod, and add the tmate-on-failure step.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The suite has never run in CI, so 239 specs across 32 files were verified only
by hand. Path-filtered to distributed code, advisory until it earns a track
record, and with flake retries at 1 rather than 5 so nondeterminism surfaces
instead of being retried away.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Three corrections from review of the previous commit.
The lock-order comment on SubscriberCount claimed no path takes s.mu and a
buffer lock together. Subscribe does exactly that, holding s.mu.RLock across
replica registrations that take buf.mu. State the rule that is actually true —
s.mu precedes any buffer lock, so counting after releasing it preserves the
order — and say what follows from it: the total is a sample, not a snapshot.
waitForLogSubscriber read as general-purpose but unblocks on the first
registered subscription. Subscribe attaches the exact-key buffer and each
replica buffer one at a time, so for a replicated model the count goes positive
while later replicas are still unattached and the race survives. Rename it
waitForSingleLogSubscriber, document that it holds only where Subscribe
resolves to one buffer, and assert on exactly 1: misuse then fails loudly on
the count rather than going quietly back to being flaky. Taking the expected
count as a parameter was the alternative, but that makes callers predict a
store-internal number and an under-count fails the same silent way as the
original bug.
The snapshot-then-subscribe race had no artifact outside a report, and review
found a second site carrying it. Mark both handlers identically, including the
point that swapping the two calls duplicates rather than drops and so is not
the fix. The race itself is left alone; this branch stays test infrastructure.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The WebSocket log handler writes its "initial" batch before it calls
Subscribe, so a line appended the instant that batch arrives lands in the
circular buffer with no subscriber to receive it. Three backend-logs specs
append exactly there and then wait out a 5s read deadline; once a gorilla
read hits its deadline the connection is unusable, so the spec cannot retry.
`--focus='Worker WebSocket log streaming' --repeat=25` failed on attempt 17
with nothing else running, which is far too often to wire into CI.
Add BackendLogStore.SubscriberCount, resolving a model ID by the same
exact-key and replica-prefix rules Subscribe uses, and have the specs poll it
until the handler has attached. Nothing in production calls it and no
assertion is weakened; the handler's own snapshot/subscribe window is left as
it is, being a production streaming question rather than a test one.
Verified with 60 repeats of the WebSocket specs and three consecutive
--randomize-all runs of the whole distributed suite, all at
--flake-attempts 1: 239 of 240 specs pass in about 80 seconds.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A failed CREATE DATABASE panics out of the assertion before closeDB runs,
leaking a pgx pool per attempt. With --flake-attempts 5 that exhausts
postgres:16-alpine's 100 connection slots, at which point the cleanup path's
own Expect fails the spec and one hiccup cascades across the suite. Scope the
admin handle so the panic unwinds through defer closeDB, and let cleanup use a
fallible tryAdminDB that reports rather than asserts.
Register DeferCleanup immediately after CREATE so a later failure cannot leave
the database behind, and warn on TestInfra that the container handles are now
suite-wide.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Starting a Postgres and a NATS container per spec cost roughly 48 minutes of
startup across the 213 specs behind SetupInfra, which is why this suite was
never wired into CI. Containers move to BeforeSuite and isolation comes from
CREATE DATABASE, which the dbName argument already described.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Use a regular (non-editable) pip/uv install so the package lands in the
venv site-packages. An editable finder records the builder source path,
which breaks after the backend is copied out of the image (#9162).
Adds a regression test (scripts/build/vllm-omni-install_test.sh) that
verifies imports survive relocation with a regular install and fail with
an editable install.
Supersedes #12040 (DCO not signed by contributor).
Assisted-by: MAKI:regolo/glm5.2
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
Show partial layer offload and CPU expert placement for llama-cpp.
Correct the documented gpu_layers default to match the backend config.
Refs #10557
Assisted-by: Codex:gpt-6
Co-authored-by: localai-org-maint-bot <306269227+localai-org-maint-bot@users.noreply.github.com>
GetImageDigest calls name.ParseReference, which cannot parse the oci://
scheme. Self-hosted registries must set the scheme to be recognised by
LooksLikeOCI, so without stripping it they silently lose upgrade detection.
Strips the prefix at the call site, matching what the other two call
sites already do (pkg/downloader/uri.go, gallery/importers/llama-cpp.go).
Supersedes #12120 (DCO not signed by contributor, branch had unrelated
history needing rebase).
Assisted-by: MAKI:regolo/glm5.2
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
The global ::selection used --color-primary-light (14% primary), which
composites to about 1.1:1 against the dark page ground — selected text
was nearly indistinguishable from unselected. Give selection its own
token in both palettes and align the CodeMirror themes with the same
strengths.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Plamen K. Kosseff <p.kosseff@gmail.com>
On shared-memory AMD APUs (e.g. Strix Halo), rocm-smi VRAM is only the
BIOS UMA carve-out while usable HIP memory lives in GTT. Query
--showmeminfo all and fold GTT when the VRAM/GTT ratio matches an APU,
preserving dedicated dGPU accounting.
Fixes#12058
Signed-off-by: lei_lei <imleilei123@gmail.com>
