Review follow-up. Twelve findings, none blocking, grouped here by what they
protect.
Panics. The handler now recovers between the WebSocket upgrade and the
hand-off, the way the peer link next door already did: net/http recovers the
panic but leaves the hijacked socket open, so without this a worker keeps a
session this replica has no entry for and will never detach. The claim gate in
Attach and reclaimOne is now released with defer, so a panic under Claim cannot
wedge one node's gate for the life of the process. SetTunnels gained the
nil-receiver guard its sibling Stop has.
Operability. A deployment with no registration token stores an empty token_hash
on every worker, so every tunnel dial 401s forever on a frontend that looks
correctly configured. That now warns at startup, logs its own line rather than
sharing the "wrong token" one, and is stated in the docs together with the fact
that setting the token later needs the workers to register again.
Authorization. A node still awaiting admin approval is refused with 403. The
rest of /api/node/ gates on nothing, but the two places that hand a node
something durable, its API key and its NATS credential, both refuse a pending
one, and a tunnel is that kind of grant. Draining and unhealthy nodes keep
their tunnels on purpose.
Comments that claimed more than the code. The global auth middleware does run
on this path and then declines to reject; the future per-node secret only lands
without a change here if it lands in TokenHash; the empty-hash guard is
defensive rather than deciding; ClusterPathPrefix is no longer only
replica-to-replica; the docs no longer say a reaped replica re-claims
unconditionally.
And the test harness. SetupTestDB started a PostgreSQL container per BeforeEach
with a readiness deadline it asserted on, which is one chance per spec to fail
one spec inside its setup, anywhere, never twice in the same place: the shape of
the flake seen twice here and never reproduced. It now starts one container per
process and creates a database per call, which is the pattern tests/e2e already
proved. Isolation is unchanged and is now asserted for the first time. All 69
call sites are untouched; the eleven consumer packages run 1404 specs green, and
jobs went from 34.3s to 3.3s, agents from 13.8s to 1.9s, cluster from 97.4s to
37.5s.
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
A worker needs no inbound port: it dials GET /api/cluster/connect, the
connection becomes one multiplexed yamux session, and the frontend opens a
stream on it per request. This adds the endpoint that accepts that dial and
attaches it to the tunnel registry.
The dial is authenticated against the NODE's own stored token hash rather than
the deployment's registration token. That is the mechanism, not yet the
isolation, since a worker still registers by presenting the shared token; what
it rules out is the shortcut of comparing against the configured value, which
would have to be unpicked the day workers get their own secrets.
Every refusal happens BEFORE the WebSocket upgrade, so a dialer reads an HTTP
status rather than a handshake error. The route is registered in every
deployment, single-binary ones included, which is what puts it in front of the
route-coverage test that holds that rule in place; with no node registry it
refuses every dial, and tells a credentialed one the frontend has no cluster
rather than that its token is wrong.
A lookup that FAILED is answered as a failure. Reporting a database that could
not be read as "unauthorized" would send a worker re-registering, throwing away
the identity its tunnel and loaded models are keyed by.
Wires the tunnel registry in core/application/distributed.go and hands it to
the membership loop. Without that call the re-claim after a replica is reaped
had no production caller and could never run.
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
The gate is justified by being held for one claim round trip, and Attach
held it across the close of the session it superseded. Closing a yamux
session closes the underlying conn and then waits for both its send and
recv loops to exit, and the send loop can be inside a write bounded only
by ConnectionWriteTimeout, so that is a wait on other goroutines. It must
not stand between a worker re-dialling this node and its claim.
The gate is now released after the store and before the close, which also
makes Attach match reclaimOne, where it has always been released
explicitly on every path. This is safe because a superseded session is no
longer reachable from the map by the time it is closed: the next re-dial
replaces an entry that already names the new session.
Pin the re-claim half of the gate too. A worker that re-dials between a
re-claim's commit and its record leaves the row carrying the re-dial's
epoch while the entry carries the re-claim's, so the attachment holding
the socket releases an epoch the row does not have and the row outlives
it, with nothing to sweep it while this replica is alive. Only Attach's
half of the serialisation was asserted; keying the two apart left every
spec green.
Also take the test hook's action under the lock that guards whether it
has fired. It was written from the spec's goroutine and read from
whichever goroutine issued the statement, which is a race in the harness
that pins the serialisation specs.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Two Attach calls for one node both claim, and PostgreSQL serialises the
two upserts, but nothing ordered the two map writes against the two
commits. The entry left installed could be the one whose claim lost the
row, and its Detach then released an epoch the row does not carry, so the
release matched nothing and the row survived the socket.
Nothing swept that row. This replica is alive and heartbeating, so
ReapStale leaves its rows alone, and no reconnect is coming for a worker
that has gone. Owner kept naming this replica as the live owner of a
tunnel it no longer held, and every dialer sent here was answered
ErrNotOwner, which is the relay into a replica that cannot serve the
request that this phase exists to prevent.
Claims for one node now pass through a gate, so claim and record are
indivisible. It is per node rather than one lock over the registry, the
way PeerPool locks per peer: the claim is a database round trip, and a
slow one for a single worker must not hold up Open for every other.
Detach is not gated, because it takes no context and must never park
behind an in-flight database call, and it changes no epoch.
Reclaim takes the same gate, which makes its claim the newest one for
that node, so it records the epoch on whatever attachment is installed
rather than only on the one it listed. Refusing to record onto an
attachment that replaced the listed one would leave that row with nothing
able to release it. The interleave the gate does not cover is Detach, and
a claim whose attachment detached while it was in flight is now released
again rather than left behind.
Also: restore Start's doc comment, which SetTunnels had swallowed; keep
reaping other replicas when this one fails to rebuild its own row, rather
than skipping the sweep along with the re-claim; scope the comment about
an unnoticed dead socket to the keepalive of the session whoever accepted
the tunnel built, since the worker session config does not exist yet; and
pin the sortedness of Held, the nil-session refusal, and both re-claim
interleaves with specs.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Phase 1 left the connection fence with a table and no sockets behind it.
This adds the registry that holds them: Attach claims the node and then
stores the session, Open hands out a stream over the tunnel this replica
holds, Detach releases the claim it was handed, and Held names what this
process is carrying.
The claim is written before the session is stored. A claimant that
installs itself and only then finds it cannot claim has, for that window,
published a tunnel no row records, so Held names it while a peer asking
Owner is told the worker is connected nowhere.
ErrNotOwner is produced at one place, the map miss. It is a routing fact:
some other replica may hold that worker perfectly well. A broken socket
under a held entry is returned as itself, because answering "not held
here" would send a dialer looking elsewhere for a worker this replica is
holding.
Epochs are compared for equality and never ordered. Claim guarantees an
epoch is unique and never reissued; it does not guarantee the later claim
draws the larger number, because the sequence value on the insert path is
drawn before the row lock.
The membership loop now re-claims on re-register, which closes the hole
phase 1 named in ReapStale. A replica that stalls long enough is swept by
a peer, losing its instance row and, in the same transaction, every
connection it owned; Register rebuilds the instance row and nothing else,
so without this it serves workers that every other replica reports as
connected nowhere. Re-claiming draws a fresh epoch, so an attachment
carries two: the token Attach handed back, which is what Detach matches
and which never moves, and the epoch of the row currently held, which is
what Release is given. Collapsing them would leave the re-claimed row
outliving the socket with no caller able to remove it.
A tunnel whose session is already closed is skipped rather than claimed
back, because claiming is an upsert and would take the row from whoever
holds the worker now.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review round 1 on the joined Owner read. The behaviour was accepted; three
comments claimed more than the code delivered, one spec pinned less than its doc
promised, and one pre-existing spec ranked epochs.
instanceIsLive said every reader of instance liveness uses it, which was false:
ReapStale spelled the complement by hand. The complement is now written as
NOT (instanceIsLive), so "stale" is exactly "not live", including how each side
treats a NULL last_seen, and the sentence is true. Inverting that predicate
fails 3 reaper specs, so the routing is held.
The Select("node_connections.*") in Owner was justified by a SELECT * hazard
that cannot occur: with a join present and nothing selected, gorm expands the
model's own columns table-qualified (callbacks.BuildQuerySQL), and the suite is
green with the Select removed. It stays, because the projection should be a
property of this query, and the comment now says that instead.
Owner gained the dialect guard Claim has. now() and make_interval are
PostgreSQL, so on the SQLite single-binary path it failed with "no such
function: now", which reads as a missing migration; that regression already
shipped once in phase 1. The refusal is deliberately not ErrNoConnection: a
deployment with no cluster has no answer about ownership, and reporting absence
would let a caller conclude the worker is not connected. A spec in the
non-PostgreSQL block holds all three properties.
The new specs aged rows by ten minutes, which any window between zero and ten
minutes satisfies, so nothing tied Owner's window to the one the sweeper uses.
They now age to just past InstanceLiveness, and a sibling ages to half of it and
must still resolve. Widening the window tenfold fails 2 specs, narrowing it
tenfold fails 1; before this both were silent.
The concurrent-claim spec asserted the stored epoch was the highest handed out,
and justified it with claims drawing their epoch after the row lock, which
contradicts Claim's own doc: the insert path draws nextval while the tuple is
built. It now asserts the stored epoch is one of the epochs handed out, and
ranks nothing.
OwnerRow's doc justified the function with a sweeper that does not call it.
ReapStale deletes orphans with a set difference; the callers are this package's
specs and one e2e assertion. It says that, and states plainly that a caller
needing to know who owns a node in order to dial it wants Owner.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
OwnerRow is a bare row read of node_connections. A connection row outlives the
replica that wrote it: a replica that dies stops heartbeating, but its rows
survive until a peer's sweep removes them, which is up to InstanceLiveness plus
one InstanceHeartbeat later. For that whole window the table names a process
that is gone. The next component phase 2 builds is the relaying dialer, and a
dialer reading OwnerRow would relay into a corpse for roughly 35 seconds after
every replica death, then report the worker as unreachable when it is in fact
absent, which is the distinction the phase 1 end-to-end specs pinned.
Owner is the resolving read: one statement joining instances, returning
ErrNoConnection when the row is missing OR its owner is not live. Both cases are
one answer on purpose, since both mean no replica here holds this tunnel; they
differ only in which sweep has run. It is one statement, not a row read followed
by an instance lookup, because between two statements the owner can die and the
caller would act on an owner the second read would have rejected.
OwnerRow stays, unjoined, for readers that need the row itself, and a spec holds
the two apart: with an aged-out owner, OwnerRow still names it and Owner
refuses, so neither can quietly become the other.
The liveness predicate is now one string, instanceIsLive, shared by Live and by
Owner's join. Two spellings of one fact drift, and this drift would show as a
relay to a replica one query calls dead and another calls alive. It is
table-qualified so it is unambiguous inside the join, and the cutoff stays on
the database clock, so replica clock skew cannot widen or narrow the window.
Both mutations were run. Dropping the liveness predicate from the join fails 3
specs, the aged-owner one among them. Replacing the database clock with a
Go-side time.Now() fails 1: the aged-owner specs still pass, because the two
clocks agree on one host, and only the recorded-SQL spec sees the literal
timestamp. That is why that spec exists.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
ReapStale deleted from instances then node_connections while Deregister took
them the other way round, both inside one transaction and both running
concurrently by design: a replica shuts down while a peer sweeps it. Opposite
orders let each hold the row the other waits for. PostgreSQL breaks the cycle by
aborting one side, so the cost today is a warning rather than lost data, but the
inversion costs nothing to remove.
Deregister now deletes the instance row first. That is the order ReapStale is
forced into anyway, since its connection delete asks which instance rows
survived, so the sweeper is the fixed side. Both functions say the order is
deliberate and shared, and name the other. A spec records the statements each
path issues and asserts they delete from the same two tables in the same order;
racing two transactions until they really deadlock would be flaky and could pass
for the wrong reason.
The rest is comment and spec accuracy, deferred from the phase 1 task reviews:
- co-location does not imply loopback. Compose's usual host=postgres resolves to
a bridge address and discovery works there; it is a DSN that NAMES localhost
that yields a loopback source address. Corrected in the DiscoverAdvertisedAddr
doc and in the spec comment that repeated it.
- unroutableReason labelled every scoped address "link-local", including the
class the check exists for, and formatted the IP with %s, which drops the
%iface, so the reported address was not the one being rejected. Split into two
cases, both rendered with their zone. CheckAdvertisedAddr passed zone "" and
net.ParseIP rejects fe80::1%eth0, so a scoped literal looked like a name and
collected no warning at all; the zone is now split off before parsing.
- Splice's "Both callers satisfy it" claimed callers that still do not exist.
It now names the two stream types the wake-on-Close property was verified
against and says a phase 2 caller over anything else has to check it.
- restored, short, why a socket-level ECONNRESET stays reported while a yamux
reset does not: the yamux endings are the teardown Splice's own Close
provokes, and whether an aborted request is routine is the relay's policy.
- the real-yamux spec's far.Read had no deadline, so a stall parked the suite
rather than failing it.
- gorilla's SetWriteDeadline is conn.go:796, not 787.
- ClusterPathPrefix is no longer derived from: the peer route spells its path
out, because core/services/cluster must not import core/http/auth. The comment
now points at the spec that holds them together instead of claiming a
derivation the move removed.
- the epoch spec asserted e2 > e1, an ordering Claim's doc tells callers not to
rely on. It asserts uniqueness, which is what the fence guarantees, and is
named for that. A sibling spec still described the epoch as incrementing in
SQL when it is drawn from a sequence.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review found the recurring class: assertions that a wrong implementation
also satisfies.
The "refuse promptly, never park the peer" guarantee was stated in three
places and tested in none. Removing the Close from the no-relay branch left
the whole cluster suite green, because the specs asserted only that some
error arrived and yamux reports a read deadline as ErrTimeout: a parked
stream satisfied that as well as a refused one. Both specs now require an
ENDING, EOF or a reset, inside a deadline short enough that parking is
unmistakable, and both go red when the Close is removed.
Deregistration existed only in a comment. Membership.Stop ended the loop and
left the row behind, so every clean rolling restart had peers dialling a
corpse for the full liveness window; the shutdown comment described the
opposite. Registry.Deregister deletes the row and the connections that
replica owned, in one transaction, for the reason the sweeper does both, and
an e2e spec pins departure inside a budget shorter than the liveness window
so it cannot pass on the sweeper doing the work. Before: the spec times out
with both replicas still live. After: 3.6s.
The configured advertised address bypassed every check discovery makes, so
the one value most likely to be copied between hosts, 127.0.0.1, was taken
verbatim and would make every peer dial itself. Both paths now share one
rejection rule: unparseable is refused, "this host" is warned about once and
honoured, because a single-host deployment uses it correctly.
Two comments claimed more than the code does. The sweeper said a stalled
replica recovers via re-register; only its instance row does, while the
connections another replica reaped stay gone and the sockets stay held here
- phase 2 must re-claim, on re-register, every connection a replica still
holds locally. And Owner became OwnerRow, documenting that the owner it
names may be dead for up to InstanceLiveness plus a heartbeat and that any
caller acting on it must join instances itself, so the deferred constraint
lives at the call site rather than in a report; the plain name is left free
for the joining version.
Minors: warn once when the peer link mounts with no registration token, so
an operator sees the cause rather than 401s; Stop no longer blocks forever
when Start was never called; corrected the NewRegistry migration doc and an
e2e comment that described a 6s window as "throughout".
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Tasks 1 to 5 built an instances table, a splice, both halves of a peer link
and an epoch fence, and nothing in the tree called any of it: no replica
registered, no route was mounted, no sweeper ran. Proving phase 1 end to
end therefore had to start by wiring it.
A frontend in distributed mode now publishes the address its peers dial,
heartbeats it, and sweeps replicas that stopped answering along with the
connection rows they owned, in one pass so the two can never disagree about
who is alive. It serves the peer link and owns the sessions peers dial in,
refusing streams on them until phase 2 installs a relay: a session nobody
accepts on does not fail a peer's Open, it hangs it.
The address is the one peers use, not the one the process binds, and it is
derived from the route to PostgreSQL. That derivation only holds while the
database is remote, so LOCALAI_DISTRIBUTED_ADVERTISE_ADDR sets it
explicitly and a replica that can determine neither warns and keeps
serving rather than failing to start.
Three e2e scenarios run against real local-ai processes, real PostgreSQL
and real dials: replicas publish addresses that can actually be connected
to; a sibling opens a stream over the peer link and is refused without the
cluster token; and a killed replica is reported unreachable, never absent,
loses the claim it held, and takes no worker with it. Each was verified by
mutation: eight injected defects, each failing the scenario that claims to
catch it.
Also moves RegisterClusterRoutes to core/http/routes beside every other
registrar, folds AutoMigrate and the epoch sequence into one
cluster.Migrate, and turns the peer route's auth-coverage spec into a real
assertion: it drives the request through the actual auth middleware
instead of comparing two string constants, which the old spec would have
passed even with the exemption deleted.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The NodeConnection model carried `default:now()`, which is PostgreSQL syntax
reaching the DDL, so AutoMigrate failed on the single-binary SQLite path and
took every SQLite caller of nodes.NewNodeRegistry down with it. Stamp the
database clock as an expression inside Claim instead, the way Register
already does, and leave the column plain.
CREATE SEQUENCE is Postgres-only for the same reason, so it is skipped on
another dialect, and Claim refuses that dialect outright: a fence that cannot
draw a token must say so rather than fail later as a missing function.
Also correct a claim the previous commit made in both the doc comment and its
message. An epoch is unique and never reissued, but it is not ordered: the
insert path draws its sequence value before taking the row lock, so a claim
that inserts after a release can be handed a lower number than one already
issued. Uniqueness is what Release needs, since it matches by equality;
callers must never compare epochs for order.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Release deletes the row, so a per-row `epoch + 1` restarted the numbering at 1
for the next claim. A replica could then be handed an epoch it already held:
claim w1 at epoch 1, lose the link silently, watch another replica claim and
release, reclaim and be handed 1 again, and its delayed cleanup for the first
dead link would match the live claim and delete it. The fence has to be
unique per node over time, not per row lifetime.
Every claim now draws nextval from a dedicated sequence on both the insert and
the conflict paths, so an epoch is never issued twice. The draw still happens
after the row lock on the conflict path, so the winning claim still holds the
highest epoch handed out.
Also drop last_seen. Nothing maintained it and it was always equal to
connected_at, but an indexed column named that way invites a second liveness
clock; whether the owner is alive is Instance.LastSeen, and whether a claim is
current is the epoch.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
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>
Install 404s because the gallery still points at mmproj-...-f16.gguf.
HF only ships ...-F16.gguf now, with a different sha256.
Signed-off-by: lei_lei <96427312+leilei3167@users.noreply.github.com>
Fixes#11673: on macOS the DMG launcher appeared to launch nothing. After
installing, the app sat in the menu bar with no window, nothing listening
on localhost:8080, and empty log files, because nothing ever started the
server unless the unrelated 'start on system boot' option was enabled.
- Start the LocalAI server automatically when the launcher opens and right
after a fresh install. The new auto_start_server config key defaults to
enabled and gets a settings checkbox; the legacy auto_start key was never
honored nor exposed, so every existing launcher.json carries an
unintentional false and is deliberately left behind.
- Fix the welcome window suppressing itself: its 'don't show this again'
checkbox was initialized with the inverted value, and SetChecked fired
the change callback which persisted ShowWelcome=false on the very first
showing.
- Surface auto-start failures through the systray startup-error dialog,
since there is no visible window during auto-start.
- Pass --app-version to fyne package so the app stops reporting itself as
version 0.0.0 in the About box.
- Document the first-launch flow (menu bar app, auto-start, WebUI URL) in
the macOS getting-started page.
- Repair two launcher specs that never ran in CI: a *bool matched against
BeTrue and a /tmp assertion that trips on Linux where the test tempdir
itself lives under /tmp.
Assisted-by: Claude Code:claude-fable-5
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Ettore Di Giacinto