test(distributed): correct the failure-primitive comments and guard the wipe

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>

tests/e2e/distributed/cluster/cluster.go

@@ -56,6 +56,11 @@ type Process struct {
 	// exited closes once the reaper has collected the process. Only the reaper
 	// calls Cmd.Wait, so nothing else may: a second Wait on the same Cmd races
 	// the first and corrupts ProcessState.
+	//
+	// A closed exited proves the child is gone. An open one proves nothing: it
+	// is still open for the whole interval between the child exiting and waitid
+	// collecting it, during which the child is a zombie that signal 0 reports as
+	// alive. Anything asserting on a process being dead must poll, not sample.
 	exited  chan struct{}
 	waitErr error
 }

tests/e2e/distributed/cluster/failure.go

@@ -58,6 +58,25 @@ func (c *Cluster) KillWorker(i int) error {
 // {DataPath}/.hmac_secret, and wiping it would make every session minted before
 // the restart hash to a row the restarted replica cannot find, turning a
 // failover assertion into an unexplained 401.
+//
+// The wipe also destroys state that nothing can rebuild. This harness sets no
+// LOCALAI_STORAGE_URL, so the distributed object store is a directory under
+// {DataPath} (core/application/distributed.go:146), and quantization and
+// fine-tune jobs write their outputs to {DataPath}/quantization and
+// {DataPath}/fine-tune (core/services/{quantization,finetune}/service.go:95);
+// agent state, router-corpus, the voiceprofile store and {DataPath}/traces go
+// the same way. Postgres keeps the job row, the artifact it points at is gone.
+// So a spec that finishes a quantization or fine-tune on a replica, restarts
+// it, and then asserts the artifact is retrievable fails for a storage reason
+// dressed up as a failover one. No spec does that today; this note is here so
+// the first one that tries does not spend a day on it.
+//
+// After StopFrontendGracefully, wait for the process to actually go
+// (Eventually(c.FrontendAlive).Should(BeFalse())) before restarting. Restart
+// terminates whatever is still running with SIGKILL, so restarting straight
+// after a SIGTERM cuts the drain short and quietly turns the rolling-update
+// case into the crash case, which is the opposite of what pairing those two
+// calls is meant to express.
 func (c *Cluster) RestartFrontend(i int) error {
 	if err := c.checkFrontendIndex(i); err != nil {
 		return err
@@ -66,13 +85,18 @@ func (c *Cluster) RestartFrontend(i int) error {
 	if old == nil {
 		return fmt.Errorf("frontend %d was never started, nothing to restart", i)
 	}
+	// frontendDataDir is relative when baseDir is empty, and this deletes it:
+	// a Cluster assembled by a future test helper without a work dir would have
+	// RemoveAll walking "frontend-N/data" under the package source directory.
+	if c.baseDir == "" {
+		return fmt.Errorf("refusing to wipe the data dir of frontend %d: cluster has no work dir", i)
+	}
 	// The old process may still be running (a restart with no preceding kill) or
 	// already dead but unreaped. terminate is idempotent, bounds its wait, and
 	// releases the log handle the replacement is about to reopen; without it the
 	// replacement races the old listener for the port and leaks a file
 	// descriptor per restart.
 	old.terminate()
-
 	if err := os.RemoveAll(c.frontendDataDir(i)); err != nil {
 		return fmt.Errorf("wiping data dir of frontend %d: %w", i, err)
 	}
@@ -93,10 +117,17 @@ func (c *Cluster) FrontendAlive(i int) bool {
 	return c.frontends[i].alive()
 }
 
-// alive reports whether the process is still running. The reaper's exited
-// channel is authoritative and is consulted first: between a child's death and
-// the reaper's Wait returning, the child is a zombie, and signal 0 to a zombie
-// succeeds, which would report a dead replica as alive.
+// alive reports whether the process is still running.
+//
+// The exited check is cheap hygiene, not a fix for the zombie window. The
+// reaper closes exited only after Cmd.Wait returns, and Wait marks the
+// os.Process done before it returns (runtime/os pidfd path), so by the time
+// exited is closed signal 0 already errors: this branch cannot fire earlier
+// than the one it precedes. The window that stays open is the other one,
+// between the child exiting and waitid collecting it: there the child is a
+// zombie, signal 0 to a zombie succeeds, and alive reports true for a process
+// that is already dead. There is no local fix; the caller's is to poll,
+// Eventually(c.FrontendAlive).Should(BeFalse()), rather than assert once.
 func (p *Process) alive() bool {
 	if p == nil || p.Cmd == nil || p.Cmd.Process == nil {
 		return false