Two things made every end-to-end result on this branch unreliable.
make test-e2e-cluster did not rebuild ./local-ai. It only checked that a
file by that name existed, so an edit that was never rebuilt ran the whole
suite against an older build while reporting on the working tree, and a
missing binary skipped every spec and exited 0 with "Test Suite Passed".
The target now depends on a new e2e-binary target, which is the plain go
build CI already used rather than make build, since that one also builds
the React UI this suite never touches. The harness carries the other half:
localAIBinary now FAILS, locally as well as under CI, when the binary is
older than the newest non-test Go source in the tree, which covers a run
started with LOCALAI_E2E_BINARY or by invoking ginkgo directly. Test files
are excluded from that scan because they compile into the ginkgo suite and
never into local-ai. The CI job drops its own build step and the env var so
that one place owns the build and it happens after protogen-go.
test-e2e-distributed was audited for the same hole and has none: ginkgo
compiles that suite from the working tree on every run and it execs no
prebuilt binary. build-mock-backend already rebuilt unconditionally.
"Worker tunnel under load" bounded the worst probe inside the bulk transfer
window against the worst probe under the empty-load window. A max over n
samples is a biased estimator when the two n differ, and here they always
do: the bulk window is by construction longer and draws several times as
many chances at an unrelated scheduling outlier. Anything loading the box
widens that gap, so the spec reddened on what else was running: 255ms
against a 161ms budget with make lint beside it, 57ms alone. It now bounds
the probe COMPLETION RATE instead, which is the statistic a serialised
session actually moves and a mean over dozens of samples in both terms, so
a uniform slowdown cancels in the ratio. Measured with make lint running:
0.97 direct and 0.60 relayed against a floor of 0.125.
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>
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 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>
Ettore Di Giacinto