amadan

carlos / platform Public

hostfiles: Present accepts what Restore writes for overlapping live entries, through the one sequence rule both share (final review A1)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude c53e60b4db4ea686c4b7b84ff259ee630d478fb7 parent 99a0092
3 files changed, +83 −24
  • internal/hostfiles/present.go +9 −5
  • internal/hostfiles/present_test.go +46 −0
  • internal/hostfiles/restore.go +28 −19
diff --git a/internal/hostfiles/present.go b/internal/hostfiles/present.go
index 29f529694..45a51f03e 100644
--- a/internal/hostfiles/present.go
+++ b/internal/hostfiles/present.go
@@ -11,10 +11,13 @@ import (
// Present reports, read-only, whether dir and envPath hold exactly what m
// names (region HA ruling 52, VerifyLocal): every live, non-skipped,
-// non-torn entry present as a regular file at its size and sha256, and
-// the env file a regular file at its platform entry's size and sha256.
-// Extra files are not checked: the app may write after P's opening, and
-// Present is called only before it.
+// non-torn entry that Restore would write present as a regular file at
+// its size and sha256, and the env file a regular file at its platform
+// entry's size and sha256. A live entry Restore supersedes (a file and an
+// entry beneath it, resolved by sequence: superseded) is not looked for,
+// so a restore that Restore reports complete is present. Extra files are
+// not checked: the app may write after P's opening, and Present is called
+// only before it.
//
// A manifest without a live env entry is not evidence that the host has
// no env file (platform items have no tombstone: StagePlatformDelete drops
@@ -24,8 +27,9 @@ import (
// manifest with no env entry, or a tombstone (which no publisher writes),
// is never present.
func Present(dir, envPath string, m Manifest, envAbsent bool) (bool, error) {
+ loser := superseded(m)
for rel, e := range m.Entries {
- if e.Deleted || e.Torn || e.Skipped != "" {
+ if e.Deleted || e.Torn || e.Skipped != "" || loser[rel] {
continue
}
ok, err := presentFile(filepath.Join(dir, filepath.FromSlash(rel)), e.Size, e.SHA256)
diff --git a/internal/hostfiles/present_test.go b/internal/hostfiles/present_test.go
index f94d2407b..4a3be4122 100644
--- a/internal/hostfiles/present_test.go
+++ b/internal/hostfiles/present_test.go
@@ -1,11 +1,15 @@
package hostfiles
import (
+ "context"
"crypto/sha256"
"encoding/hex"
"os"
"path/filepath"
+ "reflect"
"testing"
+
+ "github.com/carlosframework/platform/internal/release"
)
// Astra round 2 finding 9: after a good install, a truncated env, another
@@ -151,3 +155,45 @@ func TestPresentReadsTheManifestBack(t *testing.T) {
t.Fatalf("an env file with no EnvItem was accepted: %v %v", ok, err)
}
}
+
+// Stage B final review A1 (Astra 1): Restore resolves a live file and a
+// live entry beneath it by sequence and leaves the loser unwritten;
+// Present accepts exactly that, in both directions, through the one rule
+// (superseded). Without it VerifyLocal answers false forever and
+// EnsureLocal repairs on every pass.
+func TestPresentAcceptsWhatRestoreWroteForOverlappingEntries(t *testing.T) {
+ ctx := context.Background()
+ for name, tc := range map[string]struct {
+ stage [][2]string // in order: the later one is the newer
+ winner string
+ loser string
+ }{
+ "the file is newer than the entry beneath it": {[][2]string{{"a/b", "child"}, {"a", "file"}}, "a", "a/b"},
+ "the entry beneath is newer than the file": {[][2]string{{"a", "file"}, {"a/b", "child"}}, "a/b", "a"},
+ } {
+ t.Run(name, func(t *testing.T) {
+ s := release.NewDirStore(t.TempDir())
+ p := open(t, s)
+ for _, st := range tc.stage {
+ stage(t, p, st[0], st[1])
+ }
+ commit(t, p)
+ dir := t.TempDir()
+ m, st, err := Restore(ctx, s, pfx, dir, nil)
+ if err != nil || st.Written != 1 || !reflect.DeepEqual(st.Superseded, []string{tc.loser}) {
+ t.Fatalf("Restore: written %d, superseded %v, %v", st.Written, st.Superseded, err)
+ }
+ env := filepath.Join(t.TempDir(), "shop.example.env")
+ if ok, err := Present(dir, env, m, true); err != nil || !ok {
+ t.Fatalf("Present after Restore = %v, %v; want true", ok, err)
+ }
+ // The winner is still checked: without it the install is not whole.
+ if err := os.RemoveAll(filepath.Join(dir, filepath.FromSlash(tc.winner))); err != nil {
+ t.Fatal(err)
+ }
+ if ok, err := Present(dir, env, m, true); err != nil || ok {
+ t.Fatalf("Present without %s = %v, %v; want false", tc.winner, ok, err)
+ }
+ })
+ }
+}
diff --git a/internal/hostfiles/restore.go b/internal/hostfiles/restore.go
index 523f2ee1d..345c75e2c 100644
--- a/internal/hostfiles/restore.go
+++ b/internal/hostfiles/restore.go
@@ -120,25 +120,7 @@ func Restore(ctx context.Context, s release.Store, prefix, dir string, origins [
}
}
sort.Strings(live)
- // A live file and a live entry beneath it cannot both exist. The newer
- // one (higher Seq) wins; the other is superseded and left alone.
- isLive := make(map[string]bool, len(live))
- for _, p := range live {
- isLive[p] = true
- }
- loser := map[string]bool{}
- for _, p := range live {
- for a := path.Dir(p); a != "."; a = path.Dir(a) {
- if !isLive[a] {
- continue
- }
- if m.Entries[a].Seq > m.Entries[p].Seq {
- loser[p] = true
- } else {
- loser[a] = true
- }
- }
- }
+ loser := superseded(m)
for _, p := range live {
if loser[p] {
st.Superseded = append(st.Superseded, p)
@@ -197,6 +179,33 @@ func Restore(ctx context.Context, s release.Store, prefix, dir string, origins [
return m, st, nil
}
+// superseded is the one overlap rule for live entries: a live file and a
+// live entry beneath it cannot both exist, so the newer one (higher Seq)
+// wins and the other is superseded; equal sequences keep the deeper one.
+// Restore leaves a superseded entry unwritten and Present does not look
+// for it, so what Restore produces is exactly what Present accepts
+// (stage B final review A1). Tombstones take no part.
+func superseded(m Manifest) map[string]bool {
+ loser := map[string]bool{}
+ for p, e := range m.Entries {
+ if e.Deleted {
+ continue
+ }
+ for a := path.Dir(p); a != "."; a = path.Dir(a) {
+ above, ok := m.Entries[a]
+ if !ok || above.Deleted {
+ continue
+ }
+ if above.Seq > e.Seq {
+ loser[p] = true
+ } else {
+ loser[a] = true
+ }
+ }
+ }
+ return loser
+}
+
// ReadPlatform returns a platform-managed item's bytes, verified. A name
// the manifest does not hold is os.ErrNotExist. prefix is where m was
// loaded from; origins are as for Restore (nil or empty: prefix alone).