update: roll back what the restart actually deploys

On the deployment deploy/README.md documents, source_dir and install_dir are
the same tree and the restart command rebuilds the image from it. The
Dockerfile builds from cmd/ and internal/ and .dockerignore keeps the host
binaries out, so restoring the snapshotted binaries restored bytes nothing
reads. A bad commit therefore cost two health timeouts and two image builds
and ended in ErrRollbackFailed with an instruction to copy files back by hand,
which would not have helped either.

A deployment that rebuilds from source now has to say how the source is put
back. source_rollback "git" records the commit before the update and checks it
back out before the rollback restart. It refuses a dirty tree, because the
recorded commit does not describe one and a forced checkout would delete his
work. A build-from-source config that says nothing is refused by Validate, at
startup, rather than at the one rollback that mattered.

Also in this change, all from the same review:

  - MethodPing, the one method a locked daemon answers. Preflight passed on an
    unlocked daemon and the post-restart Presence read failed on a locked one,
    so a good update read as SHE IS PROBABLY DOWN once the env key is gone.
  - A dial failure is reported apart from a read failure. The documented
    socket is under /var/lib/docker, which a non-root operator cannot
    traverse, and "she is not answering" was the wrong diagnosis.
  - Verify refuses to run as root over a tree owned by someone else. It runs
    make build and make test in place, and root-owned artifacts break his next
    ordinary make.
  - A rollback no longer reverts config_files. That undid every config edit
    since the last apply, phraser.model_path among them.
  - The verify-failure path no longer reports rolled_back for a compile error.
  - waitHealthy caps each attempt at the remaining budget, so a 90s timeout
    cannot run to 99s.
  - tail cuts on a rune boundary. Russian test names showed the seam.
  - The claim that mavend does not import internal/update is replaced with
    what is enforced: mavend constructs no Updater and nothing can call Apply.
  - snapshot_dir inside source_dir is refused. It landed in the build context.

Found in review of #69.

internal/config/config.go

@@ -117,10 +117,14 @@ type Config struct {
 	// the update capability does not exist, which is the state to leave it in
 	// unless the operator has read internal/update's package comment.
 	//
-	// mavend never reads this block: the daemon does not import internal/update
-	// and cannot update itself. It lives here because cmd/mavupdate — a CLI the
-	// owner runs on the host, the only trigger there is — reads the same config
-	// file to find the socket it health-checks.
+	// mavend never acts on this block: it constructs no Updater and cannot
+	// update itself. Validate below is the one thing the daemon does with it, so
+	// a broken update config is caught at startup instead of on the night it is
+	// needed. That validation is also why internal/update is linked into mavend
+	// at all — linked, with no caller, which is the property that matters. The
+	// block lives here because cmd/mavupdate — a CLI the owner runs on the host,
+	// the only trigger there is — reads the same config file to find the socket
+	// it health-checks.
 	Update *update.Config `json:"update,omitempty"`
 
 	// Voice — the client↔core surface + the stt/tts modules the daemon

internal/ipc/api.go

@@ -390,6 +390,15 @@ type ModelStatusResp struct {
 	Swappable  []string `json:"swappable,omitempty"`
 }
 
+// PingResp — the answer to MethodPing. Alive is always true (the reply itself
+// is the proof); Locked says whether the daemon is still waiting for a passkey
+// assertion, which is the one state where a CoreAPI read cannot tell an
+// operator anything.
+type PingResp struct {
+	Alive  bool `json:"alive"`
+	Locked bool `json:"locked"`
+}
+
 type listTasksReq struct {
 	Status string `json:"status"` // "" all | "live" | candidate|open|done|dropped
 }

internal/ipc/client.go

@@ -75,6 +75,7 @@ var readOnlyMethods = map[Method]bool{
 	MethodMCPServers:           true,
 	MethodDayPlan:              true,
 	MethodRecentEvents:         true,
+	MethodPing:                 true,
 }
 
 // Dial connects to a core socket at path and returns a Client. The module
@@ -636,5 +637,16 @@ func (c *Client) RevertFact(ctx context.Context, key string) (int64, error) {
 	return result.NewID, nil
 }
 
+// Ping asks whether the daemon is there, and whether it is locked. It is not a
+// CoreAPI method: it touches no store, so it answers before the passkey
+// assertion that every other read waits for.
+func (c *Client) Ping(ctx context.Context) (PingResp, error) {
+	var r PingResp
+	if err := c.call(ctx, MethodPing, nil, &r); err != nil {
+		return PingResp{}, err
+	}
+	return r, nil
+}
+
 // Compile-time check: *Client satisfies CoreAPI.
 var _ CoreAPI = (*Client)(nil)

internal/ipc/server.go

@@ -486,6 +486,11 @@ type Server struct {
 	ListSpeakersFn  ListSpeakersFunc
 	ForgetSpeakerFn ForgetSpeakerFunc
 
+	// LockedFn — reports whether the daemon is in locked (pre-unlock) mode.
+	// Read by MethodPing only. Nil ⇒ not locked, which is what an embedded or
+	// test Server without the unlock dance is.
+	LockedFn func() bool
+
 	// UnlockFn — unwraps the store encryption key from the wrapped blob using
 	// the passkey PRF secret, opens the encrypted store, and wires
 	// the rest of the daemon (voice, loop, delivery). Set by the daemon when
@@ -933,6 +938,17 @@ func (s *Server) dispatch(ctx context.Context, req Request) (json.RawMessage, er
 	// directly by the daemon (StepUp / WrapKeyFn / UnlockFn), not store
 	// state, so they can never be table entries keyed on a CoreAPI method.
 	switch req.Method {
+	case MethodPing:
+		// Deliberately reaches nothing: no store, no CoreAPI, no daemon
+		// component. That is what makes it answerable in locked mode, and it is
+		// the whole point — an update that restarts her into locked mode has to
+		// be able to tell that apart from a daemon that did not come up.
+		locked := false
+		if s.LockedFn != nil {
+			locked = s.LockedFn()
+		}
+		return marshalResult(PingResp{Alive: true, Locked: locked}), nil
+
 	case MethodAssertStepUp:
 		if s.StepUp != nil {
 			return marshalResult(nil), s.StepUp(ctx)

internal/ipc/unlock_test.go

@@ -122,3 +122,39 @@ func TestLockedCheckDefaultDenies(t *testing.T) {
 		t.Error("UnlockFn never ran")
 	}
 }
+
+// A locked daemon has to be able to say it is alive. Every CoreAPI method is
+// refused before unlock, so a health check built on one of those cannot tell a
+// daemon waiting for a passkey apart from a daemon that failed to start. That
+// is what turned a good update into the manual-recovery case in
+// internal/update. MethodPing reaches no store, so it answers either way.
+func TestPingAnswersWhileLocked(t *testing.T) {
+	_, srv, cli, _ := newServerWithStore(t)
+	srv.LockedFn = func() bool { return true }
+	locked := errors.New("daemon locked")
+	srv.Check = func(_ context.Context, m Method, _ json.RawMessage) error {
+		switch m {
+		case MethodAssertStepUp, MethodUnlock, MethodPing:
+			return nil
+		default:
+			return locked
+		}
+	}
+	ctx := context.Background()
+	p, err := cli.Ping(ctx)
+	if err != nil {
+		t.Fatalf("Ping while locked: %v", err)
+	}
+	if !p.Alive || !p.Locked {
+		t.Errorf("Ping = %+v; want alive and locked", p)
+	}
+	// And the read it replaces is still refused, which is the whole point.
+	if _, err := cli.Presence(ctx); err == nil {
+		t.Error("Presence answered while locked")
+	}
+
+	srv.LockedFn = func() bool { return false }
+	if p, err := cli.Ping(ctx); err != nil || p.Locked {
+		t.Errorf("Ping after unlock = %+v, %v; want alive and not locked", p, err)
+	}
+}

internal/ipc/wire.go

@@ -63,6 +63,14 @@ const (
 	MethodListSpeakers           Method = "list_speakers"
 	MethodForgetSpeaker          Method = "forget_speaker"
 	MethodRecentEvents           Method = "recent_events"
+
+	// MethodPing — liveness, and the only method that answers in locked mode
+	// without a passkey assertion. It reaches no store, takes no arguments and
+	// returns whether the daemon is locked, so an operator tool can tell "she is
+	// up and waiting for a passkey" apart from "she is not there at all".
+	// Everything else about her state needs the store, and the store needs the
+	// key.
+	MethodPing Method = "ping"
 )
 
 // Request — one frame from module to core. Params is the JSON-encoded argument

internal/update/apply.go

@@ -41,9 +41,31 @@ func (u *Updater) Apply(ctx context.Context) (Result, error) {
 	// has no baseline to prove itself against.
 	u.log("preflight: checking the running daemon")
 	if err := u.health(ctx, u.cfg.HealthSocket); err != nil {
+		if errors.Is(err, ErrHealthDial) {
+			// Not the daemon's fault and not fixed by fixing the daemon. The
+			// usual cause is the socket path: under a docker volume it sits in
+			// /var/lib/docker, which the operator's account cannot traverse.
+			return res, err
+		}
 		return res, fmt.Errorf("%w: %v", ErrUnhealthyBefore, err)
 	}
 
+	// 0b. If the source is part of what a rollback has to put back, it must be
+	// in a state that can be described and restored. A dirty tree is neither:
+	// the commit recorded in the snapshot does not say what is deployed, and a
+	// forced checkout on the way back would delete his uncommitted work.
+	commit := u.gitHead(ctx)
+	if u.cfg.SourceRollback == "git" {
+		if commit == "" {
+			return res, fmt.Errorf("%w: source_rollback is \"git\" but %s has no readable git HEAD", ErrSourceRollback, u.cfg.SourceDir)
+		}
+		if dirty, err := u.gitDirty(ctx); err != nil {
+			return res, fmt.Errorf("%w: %v", ErrDirtyTree, err)
+		} else if dirty {
+			return res, fmt.Errorf("%w: %s", ErrDirtyTree, u.cfg.SourceDir)
+		}
+	}
+
 	// 1. Snapshot what is deployed now, BEFORE the build.
 	//
 	// The order matters and it is not the obvious one. `make build` writes its
@@ -53,7 +75,7 @@ func (u *Updater) Apply(ctx context.Context) (Result, error) {
 	// only thing standing between a bad build and a box that needs a screwdriver,
 	// so it is taken first, while the deployed bytes are still the old ones.
 	names := append(append([]string{}, u.cfg.Binaries...), u.cfg.ConfigFiles...)
-	snap, err := u.store.Save(u.cfg.InstallDir, names, u.gitHead(ctx), "pre-update")
+	snap, err := u.store.Save(u.cfg.InstallDir, names, commit, "pre-update")
 	if err != nil {
 		return res, err
 	}
@@ -68,10 +90,13 @@ func (u *Updater) Apply(ctx context.Context) (Result, error) {
 	steps, err := u.Verify(ctx)
 	res.Steps = append(res.Steps, steps...)
 	if err != nil {
-		if rerr := snap.Restore(u.cfg.InstallDir); rerr != nil {
+		// RolledBack stays false here on purpose. Nothing was installed and
+		// nothing was restarted, so there is no rollback to report; a compile
+		// error printing rolled_back=true sends the operator looking for a
+		// restart that never happened. The log line carries what was done.
+		if rerr := u.restoreBinaries(snap); rerr != nil {
 			u.log("verify failed and the artifacts could not be put back: %v — the previous ones are in %s", rerr, snap.Dir())
 		} else {
-			res.RolledBack = true
 			u.log("verify failed; the previously deployed artifacts are back in place, she was never restarted")
 		}
 		return res, err
@@ -141,10 +166,17 @@ func (u *Updater) rollback(ctx context.Context, snap Snapshot, res Result, cause
 	ctx = context.WithoutCancel(ctx)
 	res.RolledBack = true
 	u.log("rollback: restoring snapshot %s over %s", snap.ID, u.cfg.InstallDir)
-	if err := snap.Restore(u.cfg.InstallDir); err != nil {
+	if err := u.restoreBinaries(snap); err != nil {
 		u.log("rollback: RESTORE FAILED: %v", err)
 		return res, fmt.Errorf("%w: %v (after %v); the previous artifacts are in %s — copy them back by hand", ErrRollbackFailed, err, cause, snap.Dir())
 	}
+	// On a deployment that rebuilds from source, putting the binaries back is
+	// the part that changes nothing. The source is what the restart deploys, so
+	// it goes back too, and it goes back before the restart that reads it.
+	if err := u.restoreSource(ctx, snap); err != nil {
+		u.log("rollback: SOURCE CHECKOUT FAILED: %v", err)
+		return res, fmt.Errorf("%w: %v (after %v); the tree is still on the new commit, so a restart would redeploy it — `git -C %s checkout --force %s` by hand", ErrRollbackFailed, err, cause, u.cfg.SourceDir, snap.Commit)
+	}
 	// A restore with no restart leaves the failed process running, so a failed
 	// restart here is still the manual-recovery case.
 	if err := u.restart(ctx, &res); err != nil {
@@ -197,6 +229,48 @@ func (u *Updater) restart(ctx context.Context, res *Result) error {
 	return nil
 }
 
+// restoreBinaries puts back the built artifacts and nothing else.
+//
+// ConfigFiles are snapshotted and deliberately not restored. The Config doc
+// says an update never replaces the operator's config, and a rollback that
+// quietly reverted deploy/mavend.json would undo edits made since the last
+// apply — a phraser.model_path change among them, which is how the resident
+// model gets swapped. The copies stay in the snapshot dir for him to take by
+// hand if the config is what he wants back.
+func (u *Updater) restoreBinaries(snap Snapshot) error {
+	return snap.RestoreOnly(u.cfg.InstallDir, u.cfg.Binaries)
+}
+
+// restoreSource puts the working tree back on the commit the snapshot was taken
+// at, for the deployments where that is what the restart command deploys. A
+// no-op for every other shape.
+func (u *Updater) restoreSource(ctx context.Context, snap Snapshot) error {
+	if u.cfg.SourceRollback != "git" {
+		return nil
+	}
+	if snap.Commit == "" {
+		return fmt.Errorf("snapshot %s records no commit, so there is nothing to check out", snap.ID)
+	}
+	u.log("rollback: checking %s back out to %s", u.cfg.SourceDir, snap.Commit)
+	// --force because the failed build left artifacts in the tree. Safe only
+	// because Apply refused to start on a dirty tree, so nothing uncommitted of
+	// his is in reach.
+	out, err := u.run(ctx, u.cfg.SourceDir, []string{"git", "checkout", "--force", snap.Commit})
+	if err != nil {
+		return fmt.Errorf("git checkout %s: %v: %s", snap.Commit, err, tail(out, 1000))
+	}
+	return nil
+}
+
+// gitDirty reports whether the working tree has uncommitted changes.
+func (u *Updater) gitDirty(ctx context.Context) (bool, error) {
+	out, err := u.run(ctx, u.cfg.SourceDir, []string{"git", "status", "--porcelain"})
+	if err != nil {
+		return false, fmt.Errorf("git status in %s: %v", u.cfg.SourceDir, err)
+	}
+	return strings.TrimSpace(out) != "", nil
+}
+
 // gitHead records which commit produced a snapshot, for the operator's benefit.
 // Best-effort: a tree without git is not a reason to refuse to snapshot.
 func (u *Updater) gitHead(ctx context.Context) string {

internal/update/health.go

@@ -2,6 +2,7 @@ package update
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -17,14 +18,39 @@ import (
 // Presence is the method used because it is read-only (safe to retry), needs no
 // arguments, and touches the store. It cannot write anything, so a health check
 // never leaves a trace in her memory.
+//
+// A locked daemon is healthy. With a passkey enrolled and no env key mavend
+// boots locked and refuses every CoreAPI method until an assertion arrives, so
+// a Presence read there fails for a daemon that came up perfectly. Rolling back
+// on that would turn a good update into the manual-recovery case, and the
+// rollback would boot locked too. MethodPing reaches no store and answers in
+// locked mode, so it is asked first: an answer of "locked" is proof of life and
+// is where the check stops.
 
-// DialHealth connects to the mavend socket and performs one read.
+// ErrHealthDial — the socket could not be opened at all. Kept apart from a
+// failed read because the two have different causes and different fixes: on the
+// docker deployment the socket lives under /var/lib/docker, which is
+// drwx--x--- root root, so a non-root operator gets EACCES before reaching
+// mavend. "She is not answering" would be the wrong thing to tell him.
+var ErrHealthDial = errors.New("update: cannot open the health socket")
+
+// DialHealth connects to the mavend socket and proves someone is serving it.
 func DialHealth(ctx context.Context, socket string) error {
 	c, err := ipc.Dial(socket)
 	if err != nil {
-		return fmt.Errorf("update: health dial: %w", err)
+		return fmt.Errorf("%w %s: %v", ErrHealthDial, socket, err)
 	}
 	defer c.Close()
+	// Liveness first, because it is the only question a locked daemon can
+	// answer. ErrUnknownMethod means an older mavend on the other end, which is
+	// exactly the case during a rollback to a build from before ping existed —
+	// fall through to the store read rather than calling that a failure.
+	switch p, perr := c.Ping(ctx); {
+	case perr == nil && p.Locked:
+		return nil
+	case perr != nil && !errors.Is(perr, ipc.ErrUnknownMethod):
+		return fmt.Errorf("update: health ping: %w", perr)
+	}
 	if _, err := c.Presence(ctx); err != nil {
 		return fmt.Errorf("update: health read: %w", err)
 	}
@@ -39,7 +65,20 @@ func (u *Updater) waitHealthy(ctx context.Context, timeout time.Duration) error
 	delay := 500 * time.Millisecond
 	var last error
 	for {
-		attemptCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
+		// Cap the attempt at whatever is left of the budget, not a flat 10s: an
+		// attempt starting at 89s of a 90s timeout would otherwise run to 99s,
+		// and the caller asked for 90.
+		attempt := 10 * time.Second
+		if left := deadline.Sub(u.now()); left < attempt {
+			attempt = left
+		}
+		if attempt <= 0 {
+			if last == nil {
+				last = context.DeadlineExceeded
+			}
+			return fmt.Errorf("update: not healthy after %s: %w", timeout, last)
+		}
+		attemptCtx, cancel := context.WithTimeout(ctx, attempt)
 		err := u.health(attemptCtx, u.cfg.HealthSocket)
 		cancel()
 		if err == nil {

internal/update/snapshot.go

@@ -164,11 +164,26 @@ func (st *Store) Load(id string) (Snapshot, error) {
 // This is the function the whole package exists to be able to run. It uses the
 // filesystem and nothing else — no toolchain, no build, no cooperation from the
 // code being replaced.
-func (s Snapshot) Restore(dstDir string) error {
+func (s Snapshot) Restore(dstDir string) error { return s.RestoreOnly(dstDir, nil) }
+
+// RestoreOnly is Restore limited to the named files. A nil list means all of
+// them. The caller uses it to put binaries back without putting config back:
+// see Updater.restoreBinaries.
+func (s Snapshot) RestoreOnly(dstDir string, names []string) error {
 	if s.dir == "" {
 		return errors.New("update: snapshot has no directory (load it through the store)")
 	}
+	var want map[string]bool
+	if names != nil {
+		want = make(map[string]bool, len(names))
+		for _, n := range names {
+			want[n] = true
+		}
+	}
 	for _, f := range s.Files {
+		if want != nil && !want[f.Name] {
+			continue
+		}
 		src := filepath.Join(s.dir, f.Name)
 		sum, err := hashFile(src)
 		if err != nil {

internal/update/source_test.go

@@ -0,0 +1,193 @@
+package update
+
+import (
+	"context"
+	"errors"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// The deployment the README documents builds the image from the working tree:
+// source_dir and install_dir are the same path, the Dockerfile copies cmd/ and
+// internal/ and runs the build inside the builder stage, and .dockerignore
+// keeps the host binaries out. Restoring binaries there restores bytes nothing
+// reads. These tests pin the two acceptable outcomes: the source goes back, or
+// the config is refused.
+
+// dockerBox — a fakeBox wired the way the README wires the docker deployment.
+func dockerBox(t *testing.T, sourceRollback string) (*fakeBox, Config) {
+	t.Helper()
+	b := newFakeBox(t)
+	// The source IS the deployment. The old commit's source is what the running
+	// image was built from.
+	b.byCommit["old-commit"] = "GOOD-SOURCE"
+	b.commit = "old-commit"
+	write(t, filepath.Join(b.root, "src", "source.go"), "BAD-SOURCE")
+
+	cfg := b.cfg()
+	cfg.InstallDir = cfg.SourceDir
+	cfg.ConfigFiles = nil
+	cfg.SourceRollback = sourceRollback
+	// The artifacts the docker shape snapshots live in the tree.
+	write(t, filepath.Join(b.root, "src", "mavend"), "OLD-BUILD")
+	return b, cfg
+}
+
+func TestValidate_RefusesABuildFromSourceDeploymentThatCannotRollBack(t *testing.T) {
+	_, cfg := dockerBox(t, "")
+	err := cfg.Validate()
+	if !errors.Is(err, ErrSourceRollback) {
+		t.Fatalf("Validate = %v; want ErrSourceRollback — a binary snapshot rolls back nothing when the restart rebuilds from the tree", err)
+	}
+	if _, err := New(cfg); !errors.Is(err, ErrSourceRollback) {
+		t.Fatalf("New = %v; want the same refusal", err)
+	}
+}
+
+func TestApply_RollbackPutsTheSourceBackBeforeTheRestart(t *testing.T) {
+	b, cfg := dockerBox(t, "git")
+	u, err := New(cfg, WithRunner(b.run), WithHealth(b.health))
+	if err != nil {
+		t.Fatal(err)
+	}
+	// The new build compiles and passes, and then does not come up — the case
+	// this whole package exists for.
+	first := true
+	b.healthFn = func() error {
+		if first {
+			first = false
+			return nil // preflight
+		}
+		if len(b.builtFromSource) > 0 && b.builtFromSource[len(b.builtFromSource)-1] == "GOOD-SOURCE" {
+			return nil // she answers again once the good source is deployed
+		}
+		return errors.New("she does not answer")
+	}
+
+	res, err := u.Apply(context.Background())
+	if !errors.Is(err, ErrRolledBack) {
+		t.Fatalf("Apply = %v; want ErrRolledBack", err)
+	}
+	if !res.RollbackHealthy {
+		t.Fatalf("result = %+v; want a healthy rollback", res)
+	}
+	if len(b.builtFromSource) != 2 {
+		t.Fatalf("restarts = %v; want the bad one and the rolled-back one", b.builtFromSource)
+	}
+	if b.builtFromSource[1] != "GOOD-SOURCE" {
+		t.Errorf("the rollback restarted on %q; want the previous commit's source — restoring binaries alone redeploys the bad commit", b.builtFromSource[1])
+	}
+	// And the checkout came before the restart, not after it.
+	var checkoutAt, restartAt = -1, -1
+	for i, c := range b.ran {
+		if strings.HasPrefix(c, "git checkout") {
+			checkoutAt = i
+		}
+		if c == "restart-the-thing" {
+			restartAt = i
+		}
+	}
+	if checkoutAt < 0 || restartAt < checkoutAt {
+		t.Errorf("commands ran = %v; want the checkout before the last restart", b.ran)
+	}
+}
+
+func TestApply_RefusesADirtyTreeWhenTheSourceIsTheRollbackTarget(t *testing.T) {
+	b, cfg := dockerBox(t, "git")
+	b.dirty = true
+	u, err := New(cfg, WithRunner(b.run), WithHealth(b.health))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := u.Apply(context.Background()); !errors.Is(err, ErrDirtyTree) {
+		t.Fatalf("Apply on a dirty tree = %v; want ErrDirtyTree", err)
+	}
+	for _, c := range b.ran {
+		if strings.HasPrefix(c, "make") || c == "restart-the-thing" {
+			t.Errorf("a refused update still ran %q", c)
+		}
+	}
+}
+
+func TestApply_VerifyFailureDoesNotClaimARollback(t *testing.T) {
+	b := newFakeBox(t)
+	b.testErr = errors.New("exit status 1")
+	res, err := b.updater(t).Apply(context.Background())
+	if !errors.Is(err, ErrVerifyFailed) {
+		t.Fatalf("Apply = %v; want ErrVerifyFailed", err)
+	}
+	if res.RolledBack {
+		t.Error("a compile error reported rolled_back=true; nothing was installed and nothing was restarted")
+	}
+}
+
+func TestRollback_LeavesHisConfigAlone(t *testing.T) {
+	b := newFakeBox(t)
+	if _, err := b.updater(t).Apply(context.Background()); err != nil {
+		t.Fatalf("setup Apply: %v", err)
+	}
+	// He edits the config after the update — a phraser.model_path change, say.
+	cfgPath := filepath.Join(b.root, "install", "mavend.json")
+	write(t, cfgPath, `{"tick_interval":"90s"}`)
+
+	if _, err := b.updater(t).Rollback(context.Background(), ""); err != nil && !errors.Is(err, ErrRolledBack) {
+		t.Fatalf("Rollback: %v", err)
+	}
+	if got := read(t, cfgPath); !strings.Contains(got, "90s") {
+		t.Errorf("config after rollback = %q; a rollback must not revert his config", got)
+	}
+	if got := b.deployed(); got != "OLD-BUILD" {
+		t.Errorf("deployed binary = %q; want the binaries rolled back", got)
+	}
+}
+
+func TestVerify_RefusesToBuildHisTreeAsRoot(t *testing.T) {
+	b := newFakeBox(t)
+	u := b.updater(t, WithIDs(func(string) (int, uint32, error) { return 0, 1000, nil }))
+	if _, err := u.Verify(context.Background()); !errors.Is(err, ErrRootOnHisTree) {
+		t.Fatalf("Verify as root over a uid-1000 tree = %v; want ErrRootOnHisTree", err)
+	}
+	if len(b.ran) != 0 {
+		t.Errorf("the refusal still ran %v — root-owned artifacts would break his next make", b.ran)
+	}
+	// Root's own tree is fine, and so is a normal user.
+	for _, ids := range []func(string) (int, uint32, error){
+		func(string) (int, uint32, error) { return 0, 0, nil },
+		func(string) (int, uint32, error) { return 1000, 1000, nil },
+	} {
+		if _, err := b.updater(t, WithIDs(ids)).Verify(context.Background()); err != nil {
+			t.Errorf("Verify refused a legitimate build: %v", err)
+		}
+	}
+}
+
+func TestValidate_RefusesSnapshotsInsideTheSourceTree(t *testing.T) {
+	cfg := (&fakeBox{root: t.TempDir()}).cfg()
+	cfg.SnapshotDir = filepath.Join(cfg.SourceDir, "snaps")
+	if err := cfg.Validate(); err == nil {
+		t.Error("snapshot_dir inside source_dir was accepted; it lands in the docker build context")
+	}
+	cfg = (&fakeBox{root: t.TempDir()}).cfg()
+	cfg.SourceRollback = "svn"
+	if err := cfg.Validate(); err == nil {
+		t.Error("an unknown source_rollback was accepted")
+	}
+}
+
+func TestTail_CutsOnARuneBoundary(t *testing.T) {
+	s := strings.Repeat("x", 20) + "--- FAIL: TestПривет"
+	got := tail(s, 10)
+	if !utf8ValidString(got) {
+		t.Errorf("tail(%q) = %q; a cut mid-rune shows as a replacement character", s, got)
+	}
+}
+
+func utf8ValidString(s string) bool {
+	for _, r := range s {
+		if r == 0xFFFD {
+			return false
+		}
+	}
+	return true
+}

internal/update/update.go

@@ -10,12 +10,16 @@
 //   - It is never automatic and never on a timer. There is no checker, no
 //     channel, no "check for updates" call and nothing that fires from the tick
 //     loop. Apply runs exactly when a human runs cmd/mavupdate on the box.
-//   - The daemon cannot update itself. mavend does not import this package and
-//     there is no IPC method and no web route that reaches it, so no act, no
-//     intent, no tool and no LLM output can start an update. The trigger needs
-//     shell access to the host, which is a strictly higher bar than the step-up
-//     passkey gate that guards /tools — an update is not a thing to expose to
-//     anything reachable over the network.
+//   - The daemon cannot update itself. mavend never constructs an Updater and
+//     nothing in the daemon can call Apply: there is no IPC method and no web
+//     route that reaches this package, so no act, no intent, no tool and no LLM
+//     output can start an update. (The package IS linked into mavend, via
+//     internal/config, which calls Config.Validate so a bad update block is
+//     caught at daemon startup rather than on the night it is needed. Linked is
+//     not reachable — the guarantee is the absent caller, not an absent
+//     import.) The trigger needs shell access to the host, which is a strictly
+//     higher bar than the step-up passkey gate that guards /tools — an update
+//     is not a thing to expose to anything reachable over the network.
 //   - It does not fetch code. Nothing here talks to a release server, a
 //     registry, or GitHub. The new version is whatever is in the working tree
 //     the operator points it at, which he pulled himself. Downloading and
@@ -32,13 +36,18 @@
 //
 // # The order of operations, and why
 //
-// Apply is: health-check the CURRENT daemon → build → test → snapshot → install
-// → restart → health-check → rollback on any failure.
+// Apply is: health-check the CURRENT daemon → snapshot → build → test →
+// install → restart → health-check → rollback on any failure.
 //
 // The first health check is not ceremony. If she is already not answering, a
 // failed update and a broken box are indistinguishable afterwards, and the
 // rollback has nothing to prove itself against — so Apply refuses to start.
 //
+// The snapshot comes before the build, not after, and the comment in apply.go
+// spells out why: `make build` writes into the working tree, which on the
+// docker deployment IS the install dir, so a snapshot taken after it would
+// snapshot the new artifacts.
+//
 // Build and test run BEFORE anything is written to the install dir, so a broken
 // tree costs nothing but time. Install is per-file write-temp-then-rename, so a
 // crash mid-install leaves whole files, not half ones.
@@ -49,6 +58,24 @@
 // a migration, and does not need the update to have gotten far enough to leave
 // a working anything behind.
 //
+// # What the snapshot has to cover
+//
+// A rollback is only real if it puts back the thing the restart command
+// deploys. For a bare-metal layout that is the binaries in InstallDir. For the
+// docker layout it is not: the image is built from the source tree, and the
+// host binaries never enter it. Restoring binaries there rebuilds the same bad
+// image and burns a second health timeout proving it. So a deployment that
+// rebuilds from source must say how the source is put back
+// (Config.SourceRollback), and one that cannot say is refused by Validate
+// rather than discovering it during the one rollback that mattered.
+//
+// # What an update is not
+//
+// It is not turn-safe. Nothing quiesces the daemon first: the restart kills the
+// process mid-utterance if one is in flight. The model swap in
+// internal/phraser drains, because a swap is a routine operation on a running
+// box; an update is a deliberate restart and the operator picked the moment.
+//
 // # What is out of scope on purpose
 //
 // The database is not snapshotted or rolled back. It is encrypted, live, and
@@ -84,6 +111,26 @@ var (
 	// so the previous snapshot was restored. Wraps the underlying failure.
 	ErrRolledBack = errors.New("update: rolled back")
 
+	// ErrSourceRollback — the deployment rebuilds from source, and nothing in
+	// the config says how to put the source back. Refused at Validate: see
+	// Config.SourceRollback.
+	ErrSourceRollback = errors.New("update: this deployment rebuilds from source and has no way to roll the source back")
+
+	// ErrDirtyTree — source_rollback is "git" and the working tree has
+	// uncommitted changes, so the recorded commit does not describe what is
+	// deployed and a checkout would throw work away. Refused before anything is
+	// built.
+	ErrDirtyTree = errors.New("update: the source tree has uncommitted changes — commit or stash them first")
+
+	// ErrRootOnHisTree — running as root over a tree owned by somebody else.
+	// Refused: Verify runs `make build` and `make test` in SourceDir, and as
+	// root that leaves root-owned binaries, object files and a build cache in
+	// his working tree. His next ordinary `make` then fails, so one root apply
+	// breaks the normal build. This fires easily, because the documented health
+	// socket lives under a root-only directory and sudo is the obvious way past
+	// that.
+	ErrRootOnHisTree = errors.New("update: refusing to build someone else's tree as root — it would leave root-owned artifacts and break his next make")
+
 	// ErrRollbackFailed — the worst case: the new build failed AND the restore
 	// did not bring her back. The operator has to fix the box by hand; the
 	// snapshot directory is named in the result so he knows what to copy.
@@ -105,18 +152,47 @@ type Config struct {
 	InstallDir string `json:"install_dir"`
 
 	// SnapshotDir — where the pre-install copies live. Must not be inside
-	// InstallDir: a restore reading from a directory the install is writing to
-	// is not a restore.
+	// InstallDir or SourceDir: a restore reading from a directory the install is
+	// writing to is not a restore, and a snapshot dir inside the source tree
+	// lands in the docker build context and in whatever make and git do there.
 	SnapshotDir string `json:"snapshot_dir"`
 
+	// SourceRollback — how the SOURCE is put back when the deployment rebuilds
+	// from it. "" means it is not, which is only valid when the built binaries
+	// are what gets deployed.
+	//
+	// This exists because of what a rollback has to undo, which is not always
+	// the binaries. When RestartCmd is `docker compose up -d --build`, the image
+	// is built by the Dockerfile from cmd/ and internal/, and the host binaries
+	// are excluded by .dockerignore. Restoring them then restores bytes nothing
+	// reads: the restart rebuilds the same bad image from the same bad source,
+	// and the box stays down through two health timeouts for no reason.
+	//
+	// "git" makes the source part of the snapshot: the commit is recorded before
+	// the update and a rollback checks it back out before restarting. It
+	// requires a clean tree, because a recorded commit does not describe a dirty
+	// one and a forced checkout would throw uncommitted work away.
+	//
+	// Validate refuses a build-from-source deployment (SourceDir == InstallDir)
+	// that leaves this empty, rather than letting the operator find out during
+	// the one rollback he needed.
+	SourceRollback string `json:"source_rollback,omitempty"`
+
 	// Binaries — the artifact names to snapshot and install, relative to
 	// SourceDir (built) and InstallDir (deployed). Listed explicitly rather than
 	// globbed so a stray file in the tree never gets deployed.
 	Binaries []string `json:"binaries"`
 
 	// ConfigFiles — extra files to snapshot alongside the binaries, relative to
-	// InstallDir. Snapshotted, never overwritten by an install: the operator's
-	// config is not something an update gets to replace.
+	// InstallDir. Snapshotted and never written back: not by an install, and
+	// not by a rollback either. The operator's config is not something an update
+	// gets to replace, and a rollback that reverted it would silently undo every
+	// edit since the last apply. The copies are in the snapshot dir if he wants
+	// one back.
+	//
+	// The exception is source_rollback "git": a checkout moves every tracked
+	// file, config included. That is the same rollback the deployment needs to
+	// work at all, so on that shape a config edit belongs in a commit.
 	ConfigFiles []string `json:"config_files,omitempty"`
 
 	// RestartCmd — how this deployment restarts mavend, e.g.
@@ -156,6 +232,17 @@ func (c Config) Validate() error {
 	if within(c.SnapshotDir, c.InstallDir) {
 		return fmt.Errorf("update: snapshot_dir %q is inside install_dir %q — a restore must not read from what the install writes", c.SnapshotDir, c.InstallDir)
 	}
+	if within(c.SnapshotDir, c.SourceDir) {
+		return fmt.Errorf("update: snapshot_dir %q is inside source_dir %q — snapshots would land in the build context, and in whatever make and git do to that tree", c.SnapshotDir, c.SourceDir)
+	}
+	switch c.SourceRollback {
+	case "", "git":
+	default:
+		return fmt.Errorf("update: source_rollback %q is not a thing — use \"git\" or leave it out", c.SourceRollback)
+	}
+	if c.buildsFromSource() && c.SourceRollback == "" {
+		return fmt.Errorf("%w: source_dir and install_dir are both %q, so the restart deploys the tree and a restore of the binaries would undo nothing. Set \"source_rollback\": \"git\", or split the layout so install_dir holds what actually runs", ErrSourceRollback, c.SourceDir)
+	}
 	if len(c.Binaries) == 0 {
 		return errors.New("update: binaries is empty — nothing to install")
 	}
@@ -186,6 +273,13 @@ func (c Config) withDefaults() Config {
 	return c
 }
 
+// buildsFromSource — the deployment whose restart command rebuilds from the
+// tree, which is what SourceDir == InstallDir means in practice (install is a
+// no-op copy and the artifacts that matter are produced inside the image).
+func (c Config) buildsFromSource() bool {
+	return filepath.Clean(c.SourceDir) == filepath.Clean(c.InstallDir)
+}
+
 func (c Config) healthTimeout() time.Duration {
 	return time.Duration(c.HealthTimeoutSec) * time.Second
 }
@@ -234,6 +328,9 @@ type Updater struct {
 	health HealthCheck
 	log    Logger
 	now    func() time.Time
+	// ids reports the running euid and the owner of a directory. Injected so
+	// the root-build refusal is testable without a second account.
+	ids func(dir string) (int, uint32, error)
 }
 
 // New builds an Updater. Every seam has a real default; the tests replace them.
@@ -248,6 +345,7 @@ func New(cfg Config, opts ...Option) (*Updater, error) {
 		health: DialHealth,
 		log:    func(string, ...any) {},
 		now:    time.Now,
+		ids:    realIDs,
 	}
 	for _, o := range opts {
 		o(u)
@@ -266,5 +364,10 @@ func WithClock(f func() time.Time) Option {
 	return func(u *Updater) { u.now = f }
 }
 
+// WithIDs replaces the euid/owner lookup behind the root-build refusal.
+func WithIDs(f func(dir string) (int, uint32, error)) Option {
+	return func(u *Updater) { u.ids = f }
+}
+
 // Snapshots lists what is available to roll back to, newest first.
 func (u *Updater) Snapshots() ([]Snapshot, error) { return u.store.List() }

internal/update/update_test.go

@@ -32,11 +32,24 @@ type fakeBox struct {
 	healthErrs   int // remaining failures to serve
 	healthy      bool
 	healthChecks int
+	// healthFn overrides the scripted behaviour entirely, for tests whose
+	// verdict depends on what the last restart actually deployed.
+	healthFn func() error
 	// deployedAtRestart records the installed bytes each time restart runs, so a
 	// test can prove the rollback put the old bytes back BEFORE restarting.
 	deployedAtRestart []string
+	// builtFromSource records the source the restart would have built an image
+	// from, each time it runs.
+	builtFromSource []string
 
 	ran []string
+
+	// The git half, for the deployment whose restart rebuilds from the tree.
+	// commit is HEAD; byCommit is what each commit's source says; dirty makes
+	// `git status --porcelain` report uncommitted work.
+	commit   string
+	byCommit map[string]string
+	dirty    bool
 }
 
 func newFakeBox(t *testing.T) *fakeBox {
@@ -52,7 +65,11 @@ func newFakeBox(t *testing.T) *fakeBox {
 	write(t, filepath.Join(root, "install", "mavend.json"), `{"tick_interval":"60s"}`)
 	// The source tree already contains a stale binary; `make build` overwrites it.
 	write(t, filepath.Join(root, "src", "mavend"), "STALE")
-	return &fakeBox{t: t, root: root, newBytes: "NEW-BUILD", healthy: true}
+	return &fakeBox{
+		t: t, root: root, newBytes: "NEW-BUILD", healthy: true,
+		commit:   "cafebabecafebabecafebabecafebabecafebabe",
+		byCommit: map[string]string{},
+	}
 }
 
 func (b *fakeBox) cfg() Config {
@@ -86,19 +103,41 @@ func (b *fakeBox) run(ctx context.Context, dir string, argv []string) (string, e
 		}
 		return "ok", nil
 	case "git rev-parse HEAD":
-		return "cafebabecafebabecafebabecafebabecafebabe\n", nil
+		return b.commit + "\n", nil
+	case "git status --porcelain":
+		if b.dirty {
+			return " M internal/router/router.go\n", nil
+		}
+		return "", nil
 	case "restart-the-thing":
 		b.deployedAtRestart = append(b.deployedAtRestart, read(b.t, filepath.Join(b.root, "install", "mavend")))
+		// The docker shape: the restart rebuilds the image from the tree, so
+		// what it deploys is the source, not any binary on the host.
+		if src, err := os.ReadFile(filepath.Join(b.root, "src", "source.go")); err == nil {
+			b.builtFromSource = append(b.builtFromSource, string(src))
+		}
 		if b.restartErr != nil {
 			return "no such container", b.restartErr
 		}
 		return "restarted", nil
 	}
+	if len(argv) == 4 && argv[0] == "git" && argv[1] == "checkout" && argv[2] == "--force" {
+		content, ok := b.byCommit[argv[3]]
+		if !ok {
+			return "error: pathspec did not match", errors.New("exit status 1")
+		}
+		b.commit = argv[3]
+		write(b.t, filepath.Join(b.root, "src", "source.go"), content)
+		return "HEAD is now at " + argv[3], nil
+	}
 	return "", errors.New("unexpected command: " + strings.Join(argv, " "))
 }
 
 func (b *fakeBox) health(ctx context.Context, socket string) error {
 	b.healthChecks++
+	if b.healthFn != nil {
+		return b.healthFn()
+	}
 	if b.healthErrs > 0 {
 		b.healthErrs--
 		return errors.New("connection refused")

internal/update/verify.go

@@ -2,8 +2,12 @@ package update
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os"
+	"syscall"
 	"time"
+	"unicode/utf8"
 )
 
 // Verification is "does this tree build and does it pass its own tests", run
@@ -34,6 +38,9 @@ type Step struct {
 
 // Verify runs the build and the test suite in SourceDir.
 func (u *Updater) Verify(ctx context.Context) ([]Step, error) {
+	if err := u.refuseRootBuild(); err != nil {
+		return nil, err
+	}
 	ctx, cancel := context.WithTimeout(ctx, u.cfg.verifyTimeout())
 	defer cancel()
 	var steps []Step
@@ -55,11 +62,43 @@ func (u *Updater) Verify(ctx context.Context) ([]Step, error) {
 	return steps, nil
 }
 
+// refuseRootBuild stops a sudo'd apply from building in a tree it does not own.
+//
+// The seam is injected so the tests can drive both sides without a second uid.
+func (u *Updater) refuseRootBuild() error {
+	uid, owner, err := u.ids(u.cfg.SourceDir)
+	if err != nil || uid != 0 || owner == 0 {
+		return nil // not root, or root's own tree, or we cannot tell
+	}
+	return fmt.Errorf("%w: %s is owned by uid %d", ErrRootOnHisTree, u.cfg.SourceDir, owner)
+}
+
+// realIDs — the running uid and the owner of dir. Split out for the tests.
+func realIDs(dir string) (uid int, owner uint32, err error) {
+	fi, err := os.Stat(dir)
+	if err != nil {
+		return 0, 0, err
+	}
+	st, ok := fi.Sys().(*syscall.Stat_t)
+	if !ok {
+		return 0, 0, errors.New("update: cannot read directory ownership")
+	}
+	return os.Geteuid(), st.Uid, nil
+}
+
 // tail keeps the last n bytes — a failing `make test` prints far more than is
 // useful, and the failure is always at the end.
+//
+// The cut is nudged forward to a rune boundary. Russian test names and fixture
+// strings are the common case in this tree, and a slice landing mid-rune starts
+// the log with a replacement character.
 func tail(s string, n int) string {
 	if len(s) <= n {
 		return s
 	}
-	return "…" + s[len(s)-n:]
+	cut := len(s) - n
+	for cut < len(s) && !utf8.RuneStart(s[cut]) {
+		cut++
+	}
+	return "…" + s[cut:]
 }