Commit Graph
9 Commits
Author SHA1 Message Date
Ettore Di Giacinto 95cb3cfa41 fix(distributed): follow current backend protocol
Master added Animate3D, negative_prompt, and context_size after this
branch diverged. The old suite did not exercise those paths, and Kokoros
no longer implemented the generated service trait.

Extend binary conformance across the tunnel owner and peer relay. Allow
long development versions so rebased binaries can register in PostgreSQL.
Clear the security findings introduced by the branch's new code.

Assisted-by: Codex:GPT-5 [apply_patch] [exec_command]
2026-09-20 03:05:35 +00:00
Ettore Di Giacinto e25293f7bc feat(cluster): make a peer prove which replica it is
GET /api/cluster/peer authenticated with the deployment's shared
registration token and took the dialling replica's id from ?id= on trust.
Every worker holds that token, so anything holding it could open a peer
link as any replica: relay through it to every worker tunnel that replica
owns, displace a real replica's inbound link by declaring its id, and
point the roughly 31 GiB per-session receive window at one replica.

Validating the id against the instances table does not fix this, because
the attack declares a real replica's id. So the route now checks two
credentials and needs both. The shared token still says the dialler
belongs to this deployment; a new per-replica credential says which
replica it is.

The credential follows the per-node worker credential rather than
inventing a second mechanism: crypto/rand.Text, stored only as a hex
SHA-256, compared in constant time, with no fallback to the shared token.
It differs in the stronger direction. A worker's credential is minted by
the frontend and handed over once; a replica writes its own instances
row, so it mints its own secret, publishes only the hash in the same
statement that publishes its address, and never sends the plaintext
anywhere but the peer dial.

A peer that presents no credential is refused, not waved through. An old
replica and an attacker holding the shared token send the same request,
so accepting the first accepts the second; there is no safe downgrade
here, only a quiet one. The refusal is made loud instead, on both sides,
naming the upgrade rather than the network. On the documented
frontend-first order a new replica still dials an old one; an old replica
cannot dial a new one, which costs relayed requests that land on a
not-yet-restarted replica and surfaces as no route, never as absence.

A rejected peer gets its own sentinel, ErrPeerRejected, whose unwrap
chain carries ErrPeerUnreachable as well and no absence sentinel at all.
Keeping the older sentinel means no existing consumer changes behaviour;
the cause stays out of the chain, so absence cannot escape through it and
nothing can read an authorization failure as a worker that went away.

One consequence beyond the fix: a replica with no advertised address has
no instances row, so it now cannot dial out either. It was already
unreachable inward. The startup error and the docs say so.

Registry.Register, NewMembership, NewPeerPool, PeerHandler and
RegisterClusterRoutes all gained required arguments, so the identity
cannot be dropped without a compile failure.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-09-20 03:05:35 +00:00
Ettore Di Giacinto 06f58d0d88 feat(distributed): dispatch queued work as a claim queue
The three NATS queue groups jobs.new, jobs.mcp-ci.new and agent.execute are
gone. Dispatching work is now a row in a work_claims table, taken by one
frontend replica with SELECT ... FOR UPDATE SKIP LOCKED and driven on an agent
worker as a streaming control RPC over that worker's tunnel. Exactly-one
delivery among competing consumers is a database problem, not a broker feature.
An agent worker has no database, so it never claims; it executes what the
claiming replica hands it.

A claim must not outlive the replica that took it. The reap releases a claim
whose owner is no longer a live replica in the instances table, on the database
clock, and never asks how long the claim has been held. A job that legitimately
runs for an hour on a heartbeating replica is left alone, while a claim whose
owner stopped heartbeating becomes claimable again within one liveness window.
A replica with no advertised address has no instances row at all, so it refuses
to claim rather than have its work reaped out from under it mid-run.

The settle rule is stated once, in settleClaim, and every exit path calls it. A
transport failure releases the claim and never completes or discards it; only a
decoded reply line completes it. That line is deliberately not
cluster.IsWorkerAnswer, which accepts the stream refusals a worker's tunnel
writes before any request body reaches its control server: completing on those
would discard work that never ran.

The terminal line is persisted before the claim is completed, so a store that
refuses leaves the claim standing rather than leaving the job running for ever.
That is the dropped-result defect fixed structurally rather than by retry.

This also surfaces a pre-existing gap rather than causing one: no worker has
ever served plain task jobs, and publishing them into an empty queue group left
them running with no trace. Such a claim is now failed with a reason.

Removes QueueWorkers, --agent-subject and --agent-queue, and narrows an agent
worker's minted JWT by agent.execute and jobs.mcp-ci.new.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-09-20 03:05:34 +00:00
Ettore Di Giacinto bd1a6feab2 fix(cluster): make the ownership comments say only what holds
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>
2026-09-20 03:05:34 +00:00
Ettore Di Giacinto 458dafdae0 feat(cluster): resolve tunnel ownership against a live owner
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>
2026-09-20 03:05:34 +00:00
Ettore Di Giacinto db66c94d58 fix(cluster): share one lock order, and correct the phase 1 comments
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>
2026-09-20 03:05:34 +00:00
Ettore Di Giacinto d576af6943 fix(cluster): hold the guarantees phase 1's comments were claiming
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>
2026-09-20 03:05:34 +00:00
Ettore Di Giacinto 766edea9de fix(cluster): stamp liveness on the database clock and refuse undialable addresses
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>
2026-09-20 03:05:34 +00:00
Ettore Di Giacinto 1c81356ff5 feat(cluster): record frontend replicas in a shared instances table
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>
2026-09-20 03:05:34 +00:00