From 9f6801a6c563b38e58eac52d5e09d0d34ceec6a1 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 01:15:13 +0100 Subject: [PATCH] A module whose apply failed is no longer said to run what it ran (issue 1966, recovery) On 2026-11-08 a failed apply of distribution's new build had already replaced its store container, yet the node-engine still held the old build as running, so the put back to it at 23:47 UTC was judged unchanged and applied nothing. A failed apply now marks what runs of the module failed, and a want of the composition that ran before is applied again. Claude-Session: https://claude.ai/code/session_01NcbgfFfQ4Z9UE5jK32PmE5 --- internal/agent/agent.go | 17 +++++++++++++++- internal/agent/agent_test.go | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index d1e8bb5..d92bed2 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1372,7 +1372,9 @@ func (a *Agent) settleOne(ctx context.Context, module string) { } a.mu.Lock() r, runs := a.running[module] - runsWant := runs && r.Fp == w.Want.Fp && r.Composed == w.Want.Composed && r.Phase != nodeengineclient.PhaseRemoved + // A module whose last apply failed runs no composition whole, whatever it ran before (ranFailed). + runsWant := runs && r.Fp == w.Want.Fp && r.Composed == w.Want.Composed && r.Phase != nodeengineclient.PhaseRemoved && + r.Phase != nodeengineclient.PhaseFailed att, tried := a.attempt[module] same := tried && att.FP == w.Want.Fp && att.Composed == w.Want.Composed if same && att.Held { @@ -1598,6 +1600,9 @@ func (a *Agent) apply(ctx context.Context, module string, w wantNow, comp compNo if !res.Failed || res.Transient { delete(a.pulls, module) } + if res.Failed { + a.ranFailed(module) + } what := fmt.Sprintf("%s at %s", module, fp) if module == NodeModule { what = "the node's own composition" @@ -1662,6 +1667,16 @@ func (a *Agent) apply(ctx context.Context, module string, w wantNow, comp compNo a.writeHave(ctx, Have{Module: module, FP: fp, Composed: dig, Phase: string(nodeengineclient.PhaseRunning), Detail: res.Detail}) } +// ranFailed marks what runs of a module as failed once an apply of it failed (novox/hq issue 1966, 2026-11-08): a failed +// apply may have replaced some of its resources (distribution's store container) before it stopped, so what ran is no +// longer whole, and a want of it again (a put back) is applied rather than taken as running. Called with a.mu held. +func (a *Agent) ranFailed(module string) { + if r, ok := a.running[module]; ok && r.Phase != nodeengineclient.PhaseRemoved { + r.Phase = nodeengineclient.PhaseFailed + a.running[module] = r + } +} + func (a *Agent) remove(ctx context.Context, module string, w wantNow) { a.mu.Lock() att := a.attempt[module] diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 0b3738d..2b778d6 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -487,6 +487,45 @@ func TestAFailedApplyIsRetriedOnlyByAChangeOfEitherKey(t *testing.T) { t.Errorf("a module running its want was applied again (%d applies)", n) } }) + // Issue 1966 (2026-11-08 23:47 UTC): a failed apply of distribution's new build had replaced its store container, and + // the put back to the build that ran before applied nothing, judged as running. A module whose apply failed runs no + // composition whole: the want of what ran before is applied again. + t.Run("1966: after a failed apply, the composition that ran is applied again when wanted", func(t *testing.T) { + b := newBed(t) + stop := b.run() + defer stop() + fpA, fpB := fpOf("a"), fpOf("b") + ckA, crawA, digA := composition(t, "web", fpA, false, file("web", "x", "1")) + ckB, crawB, digB := composition(t, "web", fpB, false, file("web", "x", "2")) + b.comps.put(ckA, crawA) + b.comps.put(ckB, crawB) + wk, wrawA := wantFor(t, "web", fpA, digA, "") + _, wrawB := wantFor(t, "web", fpB, digB, "") + b.wants.put(wk, wrawA) + b.idle() + if n := len(b.appliedList()); n != 1 || b.haves.last("web").Phase != string(nodeengineclient.PhaseRunning) { + t.Fatalf("the first build was not applied and running (%d applies, have %+v)", n, b.haves.last("web")) + } + b.mu.Lock() + b.failWith = "distribution.program: the store does not answer" + b.mu.Unlock() + b.wants.put(wk, wrawB) + b.idle() + if n := len(b.appliedList()); n != 2 || b.haves.last("web").Phase != string(nodeengineclient.PhaseFailed) { + t.Fatalf("the second build's apply did not fail (%d applies, have %+v)", n, b.haves.last("web")) + } + b.mu.Lock() + b.failWith = "" + b.mu.Unlock() + b.wants.put(wk, wrawA) // the put back + b.idle() + if n := len(b.appliedList()); n != 3 { + t.Fatalf("the put back to the build that ran applied nothing (%d applies): a failed apply left it said as running", n) + } + if h := b.haves.last("web"); h.FP != fpA || h.Composed != digA || h.Phase != string(nodeengineclient.PhaseRunning) { + t.Errorf("after the put back the have is %+v, want the first build running", h) + } + }) // design 51 (ENG-8, ENG-11): a pull from a store that failed is tried again when the store's holder settles, once per // settled, never on a clock; the have says whom it waits on and what triggered the last retry. t.Run("a pull from a store waits for its holder to settle", func(t *testing.T) {