Keep a replacement owed after a busy wait, and clamp a wait from a later clock

A restart-on trigger that is no file counts as changed only in its own pass, so after a busy wait the container read unchanged and was never replaced, while the report stopped listing it (review of #98).
2026-10-13 23:59:58 +02:00
parent bb60e7b584
commit 695e022991
3 changed files with 129 additions and 0 deletions
@@ -2668,6 +2668,12 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
 	// (novox/hq 04-ISSUES/009). Recreating is how a container gets restart-on, which a service
 	// already has.
 	reasons := restartedBy(r.RestartOn, changed)
+	// **A replacement owed since a busy wait began is still owed** (novox/hq ADR 0335): a restart-on trigger
+	// counts as changed only in the pass it changed, so a container left running then would otherwise read
+	// unchanged at the next pass, never be replaced, and drop from the report as if it had been.
+	if owed := owedSinceBusy(r, previous); owed != "" {
+		reasons = append(reasons, owed)
+	}

 	// What this container was created reading, as last recorded. By its declared id first, and by
 	// its NAME when that id has no record: the bundle's `store` becomes the postgres module's
@@ -2718,6 +2724,8 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
 	// stop, so a container this apply found stopped by a window is one whose window it can see.
 	if win, held := in.windows.holding(r.Name, time.Now()); held {
 		out.reads = wasReading
+		// A replacement owed since a busy wait stays owed through the window (novox/hq ADR 0335).
+		out.busySince = previous.BusySince
 		out.Action = heldStill
 		out.Detail = fmt.Sprintf("held still by the maintenance window of %s since %s; left as it is, "+
 			"and converged by the first apply after the window closes (novox/hq issue 224)",
@@ -66,6 +66,15 @@ func (b BusyWait) Words() string {
 		"waiting since %s (novox/hq ADR 0335 §2)%s", b.Until.UTC().Format(time.RFC3339), since, said)
 }

+// owedSinceBusy is the reason a container still owes the replacement it was left running for, or empty: its
+// record keeps when its busy wait began, under the same name.
+func owedSinceBusy(r *declaration.Container, previous store.Applied) string {
+	if previous.BusySince == nil || previous.Target != r.Name {
+		return ""
+	}
+	return "the replacement owed since its busy wait began at " + previous.BusySince.UTC().Format(time.RFC3339)
+}
+
 // busyClock is the clock the bound is read on. A test moves it.
 var busyClock = time.Now

@@ -84,6 +93,11 @@ func askBeforeReplacing(ctx context.Context, r *declaration.Container, run Runne
 	waited := previous.BusySince != nil && previous.Target == r.Name
 	if waited {
 		since = previous.BusySince.UTC()
+		// A wait kept from a clock that has since gone back reads as starting now, never as later: the bound
+		// holds from no later than this pass.
+		if since.After(now) {
+			since = now
+		}
 	}
 	wait := BusyWait{ID: r.ID, Container: r.Name, Since: since, Said: said}
 	switch {
@@ -324,3 +324,110 @@ func TestBusyIsReadAndHeldToItsShape(t *testing.T) {
 		}
 	}
 }
+
+// A replacement owed to a restart-on trigger is still owed after the wait: a trigger that is no file or
+// directory (a service, an identity's or a library's change) is in no container's spec and counts as changed only
+// in the pass it changed, so the container must not read unchanged at the next pass and drop from the report as if
+// replaced (novox/hq ADR 0335).
+func TestAReplacementOwedToARestartTriggerIsMadeOnceIdle(t *testing.T) {
+	now := time.Date(2026, 10, 13, 1, 0, 0, 0, time.UTC)
+	atClock(t, &now)
+	d := parse(t, `{"declaration":1,"resources":[
+	  {"id":"build-agent.server","type":"container","name":"mesh-build-agent","image":"`+pinned+`",
+	   "restart-on":["identity:build-agent"],"busy":{"command":"mesh-builder busy"}}
+	]}`)
+	r := d.Resources[0].(*declaration.Container)
+	in := inputs{declares: map[string]string{}}
+	m := newBusyMachine()
+	m.containers["mesh-build-agent"] = containerSpec(r, in)
+	ctx := context.Background()
+
+	// The trigger moved this pass, and the agent holds a build: left running.
+	m.answer(true, "building")
+	out, err := applyContainer(ctx, r, m.run, map[string]bool{"identity:build-agent": true}, in, store.Applied{})
+	if err != nil || out.Action != waitsBusy || out.busySince == nil {
+		t.Fatalf("a busy container was not left running for its trigger: %+v %v", out, err)
+	}
+	record := store.Applied{ID: r.ID, Target: r.Name, BusySince: out.busySince}
+
+	// The next pass: nothing triggers now, and the replacement is still owed.
+	now = now.Add(time.Minute)
+	m.answer(false, "idle")
+	mark := m.mark()
+	out, err = applyContainer(ctx, r, m.run, map[string]bool{}, in, record)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := m.since(mark); !slices.Equal(got, []string{"ask mesh-build-agent", "rm mesh-build-agent", "create mesh-build-agent"}) {
+		t.Fatalf("the replacement owed to the trigger was dropped: %v (%+v)", got, out)
+	}
+	if out.busy == nil || out.busy.State != BusyReplacedIdle || out.busySince != nil {
+		t.Fatalf("the replacement does not say it was made once idle: %+v", out)
+	}
+}
+
+// A wait kept from a clock that later went back is never longer than the bound from now.
+func TestAWaitFromAClockThatWentBackIsClampedToNow(t *testing.T) {
+	now := time.Date(2026, 10, 13, 1, 0, 0, 0, time.UTC)
+	atClock(t, &now)
+	m := newBusyMachine()
+	_, state := applyBusy(t, m, twoContainers(t, "1", "1"), store.State{})
+	m.answer(true, "building")
+	_, state = applyBusy(t, m, twoContainers(t, "2", "1"), state)
+	now = now.Add(-time.Hour) // the clock went back an hour
+	report, _ := applyBusy(t, m, twoContainers(t, "2", "1"), state)
+	b, _ := busyOf(report, "build-agent.server")
+	if b.State != BusyWaiting || b.Since.After(now) || !b.Until.Equal(now.Add(15*time.Minute)) {
+		t.Fatalf("a wait from a later clock is not clamped to now: %+v (now %s)", b, now)
+	}
+}
+
+// An ask that fails — no shell in the image, a timeout, a paused container — is not busy: replaced at once.
+func TestAnAskThatFailsIsIdle(t *testing.T) {
+	m := newBusyMachine()
+	_, state := applyBusy(t, m, twoContainers(t, "1", "1"), store.State{})
+	failing := func(ctx context.Context, name string, args ...string) (string, error) {
+		if args[0] == "exec" {
+			m.mu.Lock()
+			m.calls = append(m.calls, "ask "+args[1])
+			m.mu.Unlock()
+			return "", errors.New("OCI runtime exec failed: exec: \"/bin/sh\": no such file or directory")
+		}
+		return m.run(ctx, name, args...)
+	}
+	mark := m.mark()
+	report, _, err := Apply(context.Background(), archHost(t), twoContainers(t, "2", "1"), state,
+		store.OriginDeclared, failing, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := m.since(mark); !slices.Equal(got, []string{"ask mesh-build-agent", "rm mesh-build-agent", "create mesh-build-agent"}) {
+		t.Fatalf("an ask that failed held the container: %v", got)
+	}
+	if len(report.Busy) != 0 {
+		t.Errorf("an ask that failed is said as a wait: %+v", report.Busy)
+	}
+}
+
+// A stopped container holds no work: it is replaced without being asked.
+func TestAStoppedContainerIsNotAsked(t *testing.T) {
+	m := newBusyMachine()
+	_, state := applyBusy(t, m, twoContainers(t, "1", "1"), store.State{})
+	m.answer(true, "would say busy")
+	stopped := func(ctx context.Context, name string, args ...string) (string, error) {
+		if args[0] == "container" && args[len(args)-1] == "mesh-build-agent" {
+			out, err := m.run(ctx, name, args...)
+			return strings.Replace(out, "true\t", "false\t", 1), err
+		}
+		return m.run(ctx, name, args...)
+	}
+	mark := m.mark()
+	if _, _, err := Apply(context.Background(), archHost(t), twoContainers(t, "2", "1"), state,
+		store.OriginDeclared, stopped, nil, nil); err != nil && !strings.Contains(err.Error(), "not running") {
+		t.Fatal(err)
+	}
+	got := m.since(mark)
+	if slices.Contains(got, "ask mesh-build-agent") || !slices.Contains(got, "rm mesh-build-agent") {
+		t.Fatalf("a stopped container was asked, or not replaced: %v", got)
+	}
+}