amadan

carlos / platform Public

moves: the held render's intended hashes are an append-only set (Task 9a fix round 2, render replay)

notePreRender replaced pre_render's intended hash on every attempt. If
render A was written and the agent died before the ack or the rendered
record, and the retry rendered B (changed inputs) and failed to write it,
A stayed on disk with only B recorded, and no cleaning would take it.

- pre_render.intended is now a set: each attempt's hash is added before
  its write and never replaces an earlier one; a hash already recorded adds
  nothing.
- The set is capped at maxRenderIntents (32) distinct contents: past the
  cap the render refuses before it writes, so the record stays bounded and
  every file a render placed stays accounted for (the landing's deadline
  then rolls it back).
- The target's cleaning accepts any recorded intended hash, the ack's, or
  the rendered record's.

Tests: TestRenderReplayKeepsEveryIntendedHash (A written then a crash; the
retry renders B and its write fails; A is on disk; the real cleanup parks
it), TestRenderIntentsAreBounded; the I1 tests on the new record. RED:
"intended [sha256:<B>]; want [A B]".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 380495964fcb119114dd7b543f79e55e0b5a74bf parent 6e23b56
3 files changed, +113 −14
  • internal/host/movescarries_test.go +83 −2
  • internal/host/movesinstall.go +24 −10
  • internal/host/movesrollback.go +6 −2
diff --git a/internal/host/movescarries_test.go b/internal/host/movescarries_test.go
index 1f7c85167..48ba2f828 100644
--- a/internal/host/movescarries_test.go
+++ b/internal/host/movescarries_test.go
@@ -6,6 +6,7 @@ import (
"encoding/hex"
"encoding/json"
"errors"
+ "fmt"
"os"
"path/filepath"
"testing"
@@ -167,7 +168,7 @@ func TestFailedRenderAckIsParkedByCleaning(t *testing.T) {
}
j := h.landJournal(t)
want := "sha256:" + hex.EncodeToString(sum[:])
- if pre := journalledPreRender(t, j); pre.Before != "absent" || pre.Intended != want || j.Steps[landStepRendered] != want {
+ if pre := journalledPreRender(t, j); pre.Before != "absent" || len(pre.Intended) != 1 || pre.Intended[0] != want || j.Steps[landStepRendered] != want {
t.Fatalf("the render's journal: pre %q rendered %q", j.Steps[landStepPreRender], j.Steps[landStepRendered])
}
if tc.replace != nil {
@@ -270,7 +271,7 @@ func TestRenderCrashBeforeItsRecordIsParkedByCleaning(t *testing.T) {
}
h.land(t)
j := h.landJournal(t)
- if pre := journalledPreRender(t, j); pre.Intended != "sha256:"+hex.EncodeToString(sum[:]) || pre.Before != "absent" {
+ if pre := journalledPreRender(t, j); len(pre.Intended) != 1 || pre.Intended[0] != "sha256:"+hex.EncodeToString(sum[:]) || pre.Before != "absent" {
t.Fatalf("pre_render %+v", pre)
}
if _, ok := j.Steps[landStepRendered]; ok {
@@ -298,3 +299,83 @@ func TestRenderCrashBeforeItsRecordIsParkedByCleaning(t *testing.T) {
})
}
}
+
+// TestRenderReplayKeepsEveryIntendedHash (fix round 2): render A is written
+// and the agent dies before the ack or the rendered record; the retry's
+// inputs have changed, so it renders B, journals B and fails to write it. A
+// is still the file on disk. The intended hashes are an append-only set
+// across the op's attempts, so A's stays recorded and the target's cleaning
+// still parks the file.
+func TestRenderReplayKeepsEveryIntendedHash(t *testing.T) {
+ h := &rollbackHarness{landingHarness: newLandingHarness(t)}
+ h.flip(t)
+ path := filepath.Join(h.tdir, p1Host+".config.env")
+ a, b := []byte("SECRET=a\n"), []byte("SECRET=b\n")
+ sumA, sumB := sha256.Sum256(a), sha256.Sum256(b)
+ attempt := 0
+ h.renderHook = func(before func(string) error) (appconfig.HeldRender, error) {
+ attempt++
+ if attempt == 1 {
+ if err := before(hex.EncodeToString(sumA[:])); err != nil {
+ return appconfig.HeldRender{}, err
+ }
+ if err := os.WriteFile(path, a, 0o600); err != nil {
+ return appconfig.HeldRender{}, err
+ }
+ return appconfig.HeldRender{}, errors.New("the agent died after writing A")
+ }
+ if err := before(hex.EncodeToString(sumB[:])); err != nil {
+ return appconfig.HeldRender{}, err
+ }
+ return appconfig.HeldRender{}, errors.New("writing B failed")
+ }
+ h.land(t)
+ h.land(t)
+ if attempt < 2 {
+ t.Fatalf("fixture: %d render attempts", attempt)
+ }
+ pre := journalledPreRender(t, h.landJournal(t))
+ wantA, wantB := "sha256:"+hex.EncodeToString(sumA[:]), "sha256:"+hex.EncodeToString(sumB[:])
+ if len(pre.Intended) != 2 || pre.Intended[0] != wantA || pre.Intended[1] != wantB {
+ t.Fatalf("intended %v; want [A B]", pre.Intended)
+ }
+ if got, err := os.ReadFile(path); err != nil || !bytes.Equal(got, a) {
+ t.Fatalf("fixture: the file is %q %v, want A", got, err)
+ }
+ h.cleanAfterRollback(t)
+ if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("A is still live: %v", err)
+ }
+ if got, ok := parkedLocally(t, h.tdir)[p1Host+".config.env"]; !ok || !bytes.Equal(got, a) {
+ t.Fatal("A was not parked")
+ }
+}
+
+// TestRenderIntentsAreBounded (fix round 2): the set of intended hashes is
+// capped at maxRenderIntents distinct contents; a hash already recorded adds
+// nothing, and a render past the cap is refused before it writes.
+func TestRenderIntentsAreBounded(t *testing.T) {
+ h := newLandingHarness(t)
+ h.flip(t)
+ var refused error
+ h.renderHook = func(before func(string) error) (appconfig.HeldRender, error) {
+ for i := 0; i <= maxRenderIntents; i++ {
+ sum := sha256.Sum256([]byte(fmt.Sprintf("content %d", i)))
+ if err := before(hex.EncodeToString(sum[:])); err != nil {
+ refused = err
+ return appconfig.HeldRender{}, err
+ }
+ if err := before(hex.EncodeToString(sum[:])); err != nil { // the same hash again
+ t.Fatalf("a recorded hash was refused: %v", err)
+ }
+ }
+ return appconfig.HeldRender{}, errors.New("unreachable")
+ }
+ h.land(t)
+ if refused == nil {
+ t.Fatal("no render was refused past the cap")
+ }
+ if pre := journalledPreRender(t, h.landJournal(t)); len(pre.Intended) != maxRenderIntents {
+ t.Fatalf("%d intended hashes recorded, want %d", len(pre.Intended), maxRenderIntents)
+ }
+}
diff --git a/internal/host/movesinstall.go b/internal/host/movesinstall.go
index 62f0a9a5c..124341716 100644
--- a/internal/host/movesinstall.go
+++ b/internal/host/movesinstall.go
@@ -1079,16 +1079,25 @@ func (ir *installRun) configEnvPath() string {
// preRender is the landStepPreRender record: <host>.config.env's state
// before the first held render ("absent", "sha256:<hex>" of a regular file,
-// or "other"), and the sha256 of the bytes the latest render was about to
-// place, journalled before it placed them.
+// or "other"), and the sha256 of every distinct content a render of this op
+// was about to place, each journalled before its write (fix round 2). The
+// set only grows: a retry that renders other content adds its hash and
+// never replaces an earlier one, whose file may be the one on disk.
type preRender struct {
- Before string `json:"before"`
- Intended string `json:"intended"`
+ Before string `json:"before"`
+ Intended []string `json:"intended"`
}
-// notePreRender is the held render's before-write hook: it journals the
-// intended content's sha256 (hex) ahead of the write, keeping the first
-// look at the file's earlier state (Lstat'ed, never followed).
+// maxRenderIntents caps the distinct contents one op's held renders may
+// journal. A render with a hash already recorded adds nothing; past the cap
+// the render refuses before it writes, so the record stays bounded and
+// every file a render placed stays accounted for.
+const maxRenderIntents = 32
+
+// notePreRender is the held render's before-write hook: it adds the
+// intended content's sha256 (hex) to the journal's set ahead of the write,
+// keeping the first look at the file's earlier state (Lstat'ed, never
+// followed).
func (ir *installRun) notePreRender(sha string) error {
if sha == "" {
return errors.New("moves: the held render names no content")
@@ -1118,10 +1127,15 @@ func (ir *installRun) notePreRender(sha string) error {
}
}
want := "sha256:" + sha
- if pre.Intended == want {
- return nil
+ for _, h := range pre.Intended {
+ if h == want {
+ return nil // this content's ownership is already recorded
+ }
+ }
+ if len(pre.Intended) >= maxRenderIntents {
+ return fmt.Errorf("moves: %s: %d distinct held renders of %s were journalled; the render waits (the landing's deadline rolls it back)", ir.rec.Host, len(pre.Intended), ir.g.ID)
}
- pre.Intended = want
+ pre.Intended = append(pre.Intended, want)
body, err := json.Marshal(pre)
if err != nil {
return err
diff --git a/internal/host/movesrollback.go b/internal/host/movesrollback.go
index 2adbbae67..0eac0f8e1 100644
--- a/internal/host/movesrollback.go
+++ b/internal/host/movesrollback.go
@@ -1469,8 +1469,12 @@ func (r *rollbackRun) ownership(ctx context.Context, j *moves.Journal) (rollback
var pre preRender
if err := json.Unmarshal([]byte(raw), &pre); err != nil {
log.Printf("moves: %s: the journal's %s step is unreadable (%v); it vouches for no rendered config", host, landStepPreRender, err)
- } else if strings.HasPrefix(pre.Intended, "sha256:") {
- ev.configSHA256 = append(ev.configSHA256, pre.Intended)
+ } else {
+ for _, h := range pre.Intended {
+ if strings.HasPrefix(h, "sha256:") {
+ ev.configSHA256 = append(ev.configSHA256, h)
+ }
+ }
}
}
if raw, ok := j.Steps[landStepPreInstall]; ok {