carlos / platform Public

zdd-5 final review: keep main's worker publication wording (F9)

ErrBuildNotPublished is the worker path's sentinel again, with main's
text "worker build not published before its exec", so a flag-off worker
wake whose publication fails reads to the member exactly as on main.
Plan 5's staged publication (publishBeforeExec: planned and pinned
wakes, a replanned build, the overlap successor) gets its own sentinel,
errStagedBuildNotPublished. classifyStep reads both as abandonment;
memberSafeWakeReason maps the staged one to the maintenance fence's
existing wording instead of passing raw text through.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 2 weeks ago 1328fe621a5b2fb2d14d2b73832e415e1e78aa29 parent db81e73

8 files changed, +86 −31

internal/activator/activator.go +1 −1
internal/activator/generations_file.go +7 −6
internal/activator/generations_instances_test.go +8 −8
internal/activator/logsink.go +9 −0
internal/activator/swap.go +7 −6
internal/activator/wakeplan.go +1 −1
internal/activator/wakeplan_test.go +33 −2
internal/activator/workerbuild.go +20 −7

diff --git a/internal/activator/activator.go b/internal/activator/activator.go

index 22b9518f2..ee15e54c2 100644

--- a/internal/activator/activator.go

+++ b/internal/activator/activator.go

@@ -1681,7 +1681,7 @@ func (a *Service) doWake(ctx context.Context, e *actEntry) (*wokenInstance, erro

// execs it (spec §4.2, §8.5): the entry now names it (and a plan's

// fallback), so the write puts both in root's pin set. A failed write

// refuses the wake here, before the lease: nothing is taken, nothing

- // runs, and no build is blamed (ErrBuildNotPublished).

+ // runs, and no build is blamed (errStagedBuildNotPublished).

if plan.planned() || e.pin != nil {

if err := a.publishBeforeExec(); err != nil {

return w, fmt.Errorf("%s: refusing to wake: %w", host, err)

diff --git a/internal/activator/generations_file.go b/internal/activator/generations_file.go

index 19fcce447..5d8080466 100644

--- a/internal/activator/generations_file.go

+++ b/internal/activator/generations_file.go

@@ -35,7 +35,7 @@ import (

// Every staged instance spawn (a planned wake and its fallback, a hold's

// pinned wake, an overlap successor) publishes the entry state that names

// its build BEFORE the exec, and does not exec when that write fails

-// (ErrBuildNotPublished): doWake and doClaimWake before the lease, checkPlan

+// (errStagedBuildNotPublished): doWake and doClaimWake before the lease, checkPlan

// for a replanned build, swapOverlap before its Step 3a. The entry state is

// set first (e.plan, e.pin, the operation set) and then written, rather than

// recorded as a pending build: that state already lives as long as the

@@ -210,13 +210,14 @@ func (a *Service) publishesLocked(e *actEntry) bool {

// e.pin, the operation set), so the snapshot names it. When the write fails

// the caller does not exec: root reads only what the file names, and could

// sweep or restage a build it does not name under the process. The refusal

-// is ErrBuildNotPublished, which classifyStep reads as abandonment, never a

-// failure of the build. The write error itself goes to the operator log

-// (persistGenerations), not into the returned error, which may reach a

-// member's wake event. Never call it with a.mu held.

+// is errStagedBuildNotPublished, which classifyStep reads as abandonment,

+// never a failure of the build. The write error itself goes to the operator

+// log (persistGenerations), not into the returned error, which may reach a

+// member's wake event (as the fence's wording, memberSafeWakeReason). Never

+// call it with a.mu held.

func (a *Service) publishBeforeExec() error {

if err := a.persistGenerations(); err != nil {

- return ErrBuildNotPublished

+ return errStagedBuildNotPublished

}

return nil

}

diff --git a/internal/activator/generations_instances_test.go b/internal/activator/generations_instances_test.go

index d9afc3f35..d543ed240 100644

--- a/internal/activator/generations_instances_test.go

+++ b/internal/activator/generations_instances_test.go

@@ -159,8 +159,8 @@ func TestAReplannedBuildIsPublishedBeforeItExecs(t *testing.T) {

}

if broken {

_, err := a.beginRequest(context.Background(), testRoute("one.example.com"))

- if !errors.Is(err, ErrBuildNotPublished) {

- t.Fatalf("err = %v, want ErrBuildNotPublished", err)

+ if !errors.Is(err, errStagedBuildNotPublished) {

+ t.Fatalf("err = %v, want errStagedBuildNotPublished", err)

}

if gens := b.started(); len(gens) != 0 {

t.Fatalf("spawned %+v with the replanned build unpublished", gens)

@@ -246,8 +246,8 @@ func TestAnUnpublishedBuildIsNeverSpawned(t *testing.T) {

a, b, evs := planActivator(t, WakePlan{Boot: next, Mode: ModeOverlap, FirstWake: true, Fallback: &good})

a.GenerationsFile = unwritableGenerationsFile(t)

_, err := a.beginRequest(context.Background(), testRoute("one.example.com"))

- if !errors.Is(err, ErrBuildNotPublished) {

- t.Fatalf("err = %v, want ErrBuildNotPublished", err)

+ if !errors.Is(err, errStagedBuildNotPublished) {

+ t.Fatalf("err = %v, want errStagedBuildNotPublished", err)

}

if gens := b.started(); len(gens) != 0 {

t.Fatalf("spawned %+v with the generations file unwritten", gens)

@@ -263,7 +263,7 @@ func TestAnUnpublishedBuildIsNeverSpawned(t *testing.T) {

f := newSwapFixture(t)

f.a.GenerationsFile = unwritableGenerationsFile(t)

res := f.a.Swap(context.Background(), f.attempt(ModeOverlap), f.newest)

- if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, ErrBuildNotPublished.Error()) {

+ if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, errStagedBuildNotPublished.Error()) {

t.Fatalf("Swap = %+v, want abandoned: unpublished", res)

}

if gens, raises, restores := f.b.calls(); gens != 0 || raises != restores {

@@ -282,7 +282,7 @@ func TestAnUnpublishedBuildIsNeverSpawned(t *testing.T) {

f.a.Logs = sink

f.a.GenerationsFile = unwritableGenerationsFile(t)

res := f.a.Swap(context.Background(), f.attempt(ModeHold), f.newest)

- if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, ErrBuildNotPublished.Error()) {

+ if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, errStagedBuildNotPublished.Error()) {

t.Fatalf("Swap = %+v, want abandoned: unpublished", res)

}

if gens, _, _ := f.b.calls(); gens != 0 {

@@ -298,8 +298,8 @@ func TestAnUnpublishedBuildIsNeverSpawned(t *testing.T) {

a, admits, prepares, evs := claimedPlanActivator(t, WakePlan{Boot: next, Mode: ModeOverlap, FirstWake: true, Fallback: &good})

a.GenerationsFile = unwritableGenerationsFile(t)

_, err := a.beginRequest(context.Background(), registry.Route{Host: "x.example", Kind: "instance", Hibernate: true})

- if !errors.Is(err, ErrBuildNotPublished) {

- t.Fatalf("err = %v, want ErrBuildNotPublished", err)

+ if !errors.Is(err, errStagedBuildNotPublished) {

+ t.Fatalf("err = %v, want errStagedBuildNotPublished", err)

}

if admits.Load() != 0 || prepares.Load() != 0 {

t.Fatalf("admitted %d, prepared %d: an unpublished claimed wake reached the admission", admits.Load(), prepares.Load())

diff --git a/internal/activator/logsink.go b/internal/activator/logsink.go

index 3562ac961..69aeafce4 100644

--- a/internal/activator/logsink.go

+++ b/internal/activator/logsink.go

@@ -48,6 +48,15 @@ var absPathToken = regexp.MustCompile(`(^|[\s"(=])(/[\w@.\-/]+)`)

// text itself originates in our own code (backends, materialize, lease) —

// this is a rendering pass, not a secrecy boundary.

func memberSafeWakeReason(err error) string {

+ if errors.Is(err, errStagedBuildNotPublished) {

+ // A staged instance wake refused because its build could not be

+ // published first (final review F9, flag on only). The platform

+ // holds the host closed until its own write succeeds (the sweep

+ // retries it) and then it reopens by itself, which is the

+ // maintenance fence's existing wording; the write error is the

+ // operator's (persistGenerations logs it).

+ err = ErrFenced

+ }

if errors.Is(err, errWakeUnproven) || genStartUnproven(err) {

// A wake refused while the host's last failed wake is not proven

// stopped (zero-downtime spec §7.4), or that failed wake itself: a

diff --git a/internal/activator/swap.go b/internal/activator/swap.go

index 55238eb06..2afb1d11d 100644

--- a/internal/activator/swap.go

+++ b/internal/activator/swap.go

@@ -121,12 +121,13 @@ func classifyStep(lifecycle context.Context, err error) stepOutcome {

// or as a claims validation inside the work at the exec. Nothing of

// the build ran because of it (spec §7.4).

return stepStale

- case errors.Is(err, ErrBuildNotPublished):

- // A worker spawn refused because its build could not be published

- // before the exec (ruling R19 item 3): nothing of the build ran, so

- // it is abandonment, never a failure of the build. Classified here

- // and not in verdictOf, which main's flag-off respawn reads; every

- // caller of classifyStep is flag-on (swaps, holds, planned wakes).

+ case errors.Is(err, ErrBuildNotPublished), errors.Is(err, errStagedBuildNotPublished):

+ // A spawn refused because its build could not be published before

+ // the exec (ruling R19 item 3; the worker and the staged instance

+ // sentinels, final review F9): nothing of the build ran, so it is

+ // abandonment, never a failure of the build. Classified here and not

+ // in verdictOf, which main's flag-off respawn reads; every caller of

+ // classifyStep is flag-on (swaps, holds, planned wakes).

return stepStale

}

return stepFailed

diff --git a/internal/activator/wakeplan.go b/internal/activator/wakeplan.go

index 2b60d260f..7582b73e7 100644

--- a/internal/activator/wakeplan.go

+++ b/internal/activator/wakeplan.go

@@ -82,7 +82,7 @@ func (a *Service) checkPlan(ctx context.Context, e *actEntry, rt registry.Route,

a.mu.Unlock()

// The replanned build (and its fallback) is published before it execs

// (spec §4.2, §8.5). A failed write execs neither: the wake fails with

- // ErrBuildNotPublished, which is abandonment, never a failure.

+ // errStagedBuildNotPublished, which is abandonment, never a failure.

if err := a.publishBeforeExec(); err != nil {

return next, err

}

diff --git a/internal/activator/wakeplan_test.go b/internal/activator/wakeplan_test.go

index e04a9f852..4ca6c2739 100644

--- a/internal/activator/wakeplan_test.go

+++ b/internal/activator/wakeplan_test.go

@@ -963,9 +963,12 @@ func TestAPlannedWorkerRefusedAtItsPublicationAttemptsNoSpawn(t *testing.T) {

a.Planner = func(context.Context, registry.Route) WakePlan {

return WakePlan{Boot: next, Mode: ModeOverlap, FirstWake: true, Fallback: &good}

}

+ // Step 0.8 is plan 5's staged publication, flag on only, so it carries

+ // that path's sentinel (final review F9); the worker's own exec-time

+ // publication keeps ErrBuildNotPublished (the test above).

err := a.WakeWorker(alwaysOnWorkerRoute("bot.x.internal"))

- if !errors.Is(err, ErrBuildNotPublished) {

- t.Fatalf("WakeWorker = %v, want ErrBuildNotPublished", err)

+ if !errors.Is(err, errStagedBuildNotPublished) {

+ t.Fatalf("WakeWorker = %v, want errStagedBuildNotPublished", err)

}

if n := b.workerStarts(); n != 0 {

t.Fatalf("%d worker children started for an unpublished build", n)

@@ -992,6 +995,34 @@ func TestErrBuildNotPublishedIsAbandonment(t *testing.T) {

}

+// Final review F9: the worker path keeps main's sentinel with main's text

+// (origin/main a3bf7d6c), so a worker wake whose publication fails with the

+// flag off reads to the member exactly as it did. The staged instance path

+// has a sentinel of its own: abandonment too, and its member event reads as

+// the maintenance fence's existing wording, never the raw text.

+func TestTheWorkerPublicationSentinelKeepsMainsText(t *testing.T) {

+ if got, want := ErrBuildNotPublished.Error(), "worker build not published before its exec"; got != want {

+ t.Fatalf("ErrBuildNotPublished = %q, want main's %q", got, want)

+ }

+ worker := fmt.Errorf("start bot.x.internal: %w", fmt.Errorf("%w: disk full", ErrBuildNotPublished))

+ if got, want := memberSafeWakeReason(worker), "wake failed: start bot.x.internal: worker build not published before its exec: disk full"; got != want {

+ t.Fatalf("worker member text = %q, want main's passthrough %q", got, want)

+ }

+ inst := fmt.Errorf("one.example.com: refusing to wake: %w", errStagedBuildNotPublished)

+ if errors.Is(inst, ErrBuildNotPublished) || errors.Is(worker, errStagedBuildNotPublished) {

+ t.Fatal("the worker and instance sentinels must be distinct")

+ }

+ if got := classifyStep(nil, inst); got != stepStale {

+ t.Fatalf("classifyStep(instance) = %v, want stepStale", got)

+ }

+ if got := verdictOf(inst); got != gateProceed {

+ t.Fatalf("verdictOf(instance) = %v, want unchanged (gateProceed)", got)

+ }

+ if got, want := memberSafeWakeReason(inst), memberSafeWakeReason(ErrFenced); got != want {

+ t.Fatalf("instance member text = %q, want the fence's reviewed wording %q", got, want)

+ }

// Spec §6.4, §7.4: a wake cancelled by its lifecycle (here Shutdown, as

// maintenance or a stop would) is no failure of the build: no failure

// event, no fallback, so no halted cohort and no hold-down. spawnPlanned

diff --git a/internal/activator/workerbuild.go b/internal/activator/workerbuild.go

index 4975ca77f..df2758e4e 100644

--- a/internal/activator/workerbuild.go

+++ b/internal/activator/workerbuild.go

@@ -27,13 +27,26 @@ var (

_ stagedWorkerStarter = (*SplitBackend)(nil)

)

-// ErrBuildNotPublished is a spawn refused because the build it was about to

-// exec could not be written to the generations file first (ruling R19 item

-// 3; plan 5 Task 12 for staged instance spawns, publishBeforeExec): root

-// could not see that build as referenced, so it is not run. The

-// failed-spawn handling applies, and the sweep retries the write. It is

-// never a failure of the build (classifyStep).

-var ErrBuildNotPublished = errors.New("build not published before its exec")

+// ErrBuildNotPublished is a worker spawn refused because the build it was

+// about to exec could not be written to the generations file first (ruling

+// R19 item 3): root could not see that build as referenced, so it is not

+// run. The failed-spawn handling applies, and the sweep retries the write.

+// It is never a failure of the build (classifyStep). Its text is main's

+// (final review F9): it reaches a member through a worker wake's failure

+// event with the flag off, which must read as it always did. Plan 5's

+// staged publication has its own sentinel, errStagedBuildNotPublished.

+var ErrBuildNotPublished = errors.New("worker build not published before its exec")

+// errStagedBuildNotPublished is ErrBuildNotPublished for plan 5's staged

+// publication (Task 12, publishBeforeExec): the instance spawns (a planned

+// or pinned wake, a replanned build, an overlap successor) and a planned

+// wake's step 0.8, which a planned worker wake passes too before its lease.

+// A worker's own exec-time publication (startWorker's hook) keeps

+// ErrBuildNotPublished. Abandonment as well (classifyStep), and only

+// reachable with the flag on. A member's wake event never shows its text:

+// memberSafeWakeReason maps it to the maintenance fence's wording (final

+// review F9).

+var errStagedBuildNotPublished = errors.New("staged build not published before its exec")

// beforeExecKey carries the activator's before-exec hook on a spawn's

// context (withBeforeExec).