amadan

carlos / platform Public

host: the proven origin set is sorted as prefixes, the order the carried set uses (Plan 15 switch, Astra's finding)

proveOrigins sorted the generation names before turning them into files
prefixes, while SetOrigins stores the prefixes sorted. The two orders
differ for valid names such as mv1 and mv1-old ("mv1" sorts before
"mv1-old", "g-mv1-old/" before "g-mv1/"), so after a commit and a crash
establishedOver's order-sensitive comparison of the carried set with the
proven set rejected the genuine replay. The prefixes are now sorted
after the mapping, so proveOrigins, SetOrigins and establishedOver
compare the same ordering; the Proven(set) call takes the generations
as collected.

Regression: a prepared manifest for mv1 that also references an object
under mv1-old, both recorded, established once with a crash before the
local effect; the genuine replay is accepted and writes nothing. The
counting witness compares the asked set regardless of order.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude 95443dc81b798e5135003ecec51dfbb053cfef4a parent 7ccb691
2 files changed, +64 −3
  • internal/host/movesfiles.go +5 −1
  • internal/host/movesfiles_test.go +59 −2
diff --git a/internal/host/movesfiles.go b/internal/host/movesfiles.go
index e05f9f0e8..22c9a7fe0 100644
--- a/internal/host/movesfiles.go
+++ b/internal/host/movesfiles.go
@@ -491,7 +491,6 @@ func (f *movesFiles) proveOrigins(ctx context.Context, o hostclaim.Observed, in
set = append(set, gen)
}
}
- sort.Strings(set)
proven, err := f.Proof.Proven(ctx, o, in, set)
if err != nil {
return nil, fmt.Errorf("prove the origins of %s: %w", location, err)
@@ -503,6 +502,11 @@ func (f *movesFiles) proveOrigins(ctx context.Context, o hostclaim.Observed, in
}
origins = append(origins, movesFilesPrefix(location, gen))
}
+ // The prefixes are sorted, not the generation names: the two do not
+ // order alike ("mv1" before "mv1-old", "g-mv1-old/" before "g-mv1/"),
+ // and this is the carried set SetOrigins stores and establishedOver
+ // compares (Astra's finding on the Plan 15 switch).
+ sort.Strings(origins)
return origins, nil
}
diff --git a/internal/host/movesfiles_test.go b/internal/host/movesfiles_test.go
index 067e1cae9..fd428db89 100644
--- a/internal/host/movesfiles_test.go
+++ b/internal/host/movesfiles_test.go
@@ -10,6 +10,7 @@ import (
"os"
"path/filepath"
"reflect"
+ "sort"
"strings"
"sync"
"testing"
@@ -1239,10 +1240,16 @@ func TestEstablishProvesAndCarriesOnlyReferencedOrigins(t *testing.T) {
t.Fatalf("g2 origins %v, want %v", m.Origins, want)
}
// One Proven(set) call per Establish (Plan 15): the deduplicated
- // referenced origins plus the admitted generation, in one batch.
- if len(proof.calls) != 1 || !reflect.DeepEqual(proof.calls[0], []string{"g1", "g2", "g5"}) {
+ // referenced origins plus the admitted generation, in one batch (the
+ // set's order is the caller's; the prefixes are what get sorted).
+ if len(proof.calls) != 1 {
t.Fatalf("Proven calls %v, want one call over the referenced origins and the admitted generation", proof.calls)
}
+ asked := append([]string(nil), proof.calls[0]...)
+ sort.Strings(asked)
+ if !reflect.DeepEqual(asked, []string{"g1", "g2", "g5"}) {
+ t.Fatalf("Proven asked %v, want the referenced origins and the admitted generation", proof.calls[0])
+ }
// A referenced origin the proof refuses still refuses the whole.
x.f.Proof = &countingProof{proven: map[string]bool{"g1": true, "g2": true, "g3": true}}
if err := x.f.Establish(ctx, servingObserved("g3", "g1:uk:@root:uk", "g2:uk:g1:uk", "g3:uk:g2:uk"), launchInputs("g3"), x); !errors.Is(err, hostfiles.ErrOrigin) {
@@ -1471,6 +1478,56 @@ func TestRecoveryRestartTakesTheNewestRecoveryManifest(t *testing.T) {
})
}
+// The proven set and the carried set order the same way (Astra's finding
+// on the Plan 15 switch): generation names and their files prefixes do
+// not sort alike ("mv1" before "mv1-old", but "g-mv1-old/" before
+// "g-mv1/"), so the prefixes, not the names, are sorted everywhere the
+// sets are compared. A prepared manifest for mv1 that also references an
+// object under mv1-old (a same-region retained reference) is established
+// once; a crash before the local effect leaves the key at sequence 1
+// with the carried set; the genuine replay is accepted and writes nothing.
+func TestPreparedReplayOrdersOriginsLikeTheCarriedSet(t *testing.T) {
+ x := newFilesFixture(t)
+ ctx := context.Background()
+ x.writeEnv("K=1\n")
+ oldObj := movesFilesPrefix(filesLoc, "mv1-old") + "objects/0000000000000001-" + sha([]byte("O"))
+ x.bucket.Put("data", oldObj, []byte("O"))
+ newObj := movesFilesPrefix(filesLoc, "mv1") + "objects/0000000000000000-" + sha([]byte("A"))
+ x.bucket.Put("data", newObj, []byte("A"))
+ envObj := movesFilesPrefix(filesLoc, "mv1") + "objects/0000000000000000-" + sha([]byte("K=1\n"))
+ x.bucket.Put("data", envObj, []byte("K=1\n"))
+ prepared := hostfiles.Manifest{V: hostfiles.ManifestVersion, Seq: 0,
+ Entries: map[string]hostfiles.Entry{"a.txt": {Object: newObj, SHA256: sha([]byte("A")), Size: 1, Mode: 0o600, Seq: 0}, "old.txt": {Object: oldObj, SHA256: sha([]byte("O")), Size: 1, Mode: 0o600, Seq: 1}},
+ Platform: map[string]hostfiles.Entry{hostfiles.EnvItem: {Object: envObj, SHA256: sha([]byte("K=1\n")), Size: 4, Mode: 0o600, Seq: 0}},
+ Origins: []string{movesFilesPrefix(filesLoc, "mv1-old"), movesFilesPrefix(filesLoc, "mv1")}}
+ raw := mustJSON(t, prepared)
+ key := hostfiles.ManifestKey(movesFilesPrefix(filesLoc, "mv1"))
+ version := x.bucket.Put("data", key, raw)
+ x.journal("mv1", preparedManifest{Key: key, SHA256: sha(raw), Version: version, Op: "mv1", Stamp: 7})
+ x.writeEnv("K=1\nCARLOS_INSTANCE_KEY=abc\n")
+ o := servingObserved("mv1", "cur:ie:@root:ie", "mv1:uk:cur:ie")
+ o.Claim.Op = hostclaim.Op{Kind: hostclaim.KindLanding, ID: "mv1", Phase: "probing", Exec: "uk-box", Participants: []string{"ie-box", "uk-box"}, Intent: map[string]string{"to": "uk", "source": "ie-box", "target": "uk-box"}}
+ o = x.observe(o, "mv1", "mv1-old")
+ in := launchInputs("mv1")
+ in.Prepared = true
+ x.workErr, x.workErrAt = errors.New("crash before the local effect"), 3
+ if err := x.f.Establish(ctx, o, in, x); err == nil {
+ t.Fatal("the crash was not reported")
+ }
+ x.workErr, x.workErrAt = nil, 0
+ m, found := x.manifest("mv1")
+ if !found || m.Seq != 1 {
+ t.Fatalf("after the crashed attempt: %+v %v", m, found)
+ }
+ seq1 := x.bucket.LatestID("data", key)
+ if err := x.f.Establish(ctx, o, in, x); err != nil {
+ t.Fatalf("the genuine replay: %v", err)
+ }
+ if x.bucket.LatestID("data", key) != seq1 {
+ t.Fatal("the replay wrote a new revision")
+ }
+}
+
// The instance key rule (O1'): the manifest Establish leaves carries the
// env file with exactly the bytes on disk when it runs, staged and
// committed before it returns, and no adoption item; the local adoption