Merge origin/main (8e9ac1b8: C7 reads moves intent, FDHolders dev/ino, copy-gate scope) into moves-phase1-2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5 files changed,
+182
−27
approved_moves_copy_test.go+33 −6internal/hostclaim/relocating.go+15 −10internal/hostclaim/relocating_test.go+45 −9internal/stopevidence/fds_linux.go+44 −2internal/stopevidence/fds_linux_test.go+45 −0
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index e0cebe924..65d9f062b 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -639,9 +639,17 @@ func resolveMovesScanBase(lookup func() (string, error), pinned string) (base, f |
| } |
| // movesScanBaseRef is the base for this run, logged so a failure says which. |
| +// |
| +// Where git cannot name the merge base (a CI checkout with no origin/main), |
| +// the checks that need it skip rather than diff from the pinned commit: a |
| +// stale pin turns every commit main gained since into a false finding on |
| +// every other branch. |
| func movesScanBaseRef(t *testing.T) string { |
| t.Helper() |
| base, from := resolveMovesScanBase(movesMergeBase, movesScanBase) |
| + if base == movesScanBase && from == "the pinned movesScanBase" { |
| + t.Skip("no merge base with origin/main in this checkout; the diff checks run where git can name it") |
| + } |
| t.Logf("scan base %s, from %s", base, from) |
| return base |
| } |
| @@ -919,20 +927,34 @@ func movesProductionGoFile(path string) bool { |
| // movesUncovered returns the production files in changed that listed lacks. |
| // exists says whether a path is still in the tree (a deleted file needs no |
| // entry). |
| -func movesUncovered(changed, listed []string, exists func(string) bool) []string { |
| +func movesUncovered(changed, listed []string, exists, movesCode func(string) bool) []string { |
| have := map[string]bool{} |
| for _, f := range listed { |
| have[f] = true |
| } |
| var out []string |
| for _, f := range changed { |
| - if movesProductionGoFile(f) && exists(f) && !have[f] { |
| + if movesProductionGoFile(f) && exists(f) && movesCode(f) && !have[f] { |
| out = append(out, f) |
| } |
| } |
| return out |
| } |
| +// movesCodeFile is a Go file the moves series owns or speaks through: one in |
| +// internal/moves, or one that imports it. Another branch's change to a file |
| +// with no tie to moves is not this gate's to inventory. |
| +func movesCodeFile(path string) bool { |
| + if strings.HasPrefix(path, "internal/moves/") { |
| + return true |
| + } |
| + body, err := os.ReadFile(path) |
| + if err != nil { |
| + return true // unreadable: inventory it rather than pass it |
| + } |
| + return strings.Contains(string(body), `"github.com/carlosframework/platform/internal/moves`) |
| +} |
| + |
| func TestMovesScanFilesInventoryRules(t *testing.T) { |
| exists := func(p string) bool { return p != "internal/gone.go" } |
| changed := []string{ |
| @@ -943,10 +965,15 @@ func TestMovesScanFilesInventoryRules(t *testing.T) { |
| "internal/gone.go", // deleted |
| "docs/superpowers/plans/2026-10-06-copy.md", // not Go |
| "internal/moves/listed.go", // listed |
| + "internal/host/usesmoves.go", // imports moves: needs an entry |
| + "internal/changelog/other.go", // another branch's file, no tie to moves |
| + } |
| + movesCode := func(p string) bool { |
| + return strings.HasPrefix(p, "internal/moves/") || p == "internal/host/usesmoves.go" |
| } |
| - got := movesUncovered(changed, []string{"internal/moves/listed.go"}, exists) |
| - if len(got) != 1 || got[0] != "internal/moves/new.go" { |
| - t.Errorf("uncovered = %q, want only internal/moves/new.go", got) |
| + got := movesUncovered(changed, []string{"internal/moves/listed.go"}, exists, movesCode) |
| + if len(got) != 2 || got[0] != "internal/moves/new.go" || got[1] != "internal/host/usesmoves.go" { |
| + t.Errorf("uncovered = %q, want internal/moves/new.go and internal/host/usesmoves.go", got) |
| } |
| } |
| @@ -989,7 +1016,7 @@ func TestMovesScanFilesCoverTheDiff(t *testing.T) { |
| t.Skipf("git diff %s..HEAD: %v", base, err) |
| } |
| exists := func(p string) bool { _, err := os.Stat(p); return err == nil } |
| - for _, f := range movesUncovered(strings.Fields(string(out)), movesScanFiles, exists) { |
| + for _, f := range movesUncovered(strings.Fields(string(out)), movesScanFiles, exists, movesCodeFile) { |
| t.Errorf("%s changed since %s and is not in movesScanFiles", f, base) |
| } |
| for _, f := range movesScanFiles { |
diff --git a/internal/hostclaim/relocating.go b/internal/hostclaim/relocating.go| index b5565460c..b3f27d7b5 100644 |
| --- a/internal/hostclaim/relocating.go |
| +++ b/internal/hostclaim/relocating.go |
| @@ -66,7 +66,10 @@ func ConvertMovingToRelocating(ctx context.Context, s release.Store, observed Ob |
| if opID == "" || before.Op.ID != opID { |
| return Observed{}, ErrStaleOp |
| } |
| - if before.Holder == nil || before.Op.Intent["admission_id"] == "" || before.Holder.Family != "instance" || before.Op.Kind != KindMoving || before.Op.Phase == "landing" || before.Op.Phase == "committed" { |
| + // The move's own intent, as BeginMove writes it: "source" and "target", |
| + // plus the source enrollment the request door records from the roster. |
| + source := before.Op.Intent["source"] |
| + if before.Holder == nil || !validBoxReference(source) || source == "none" || before.Op.Intent["source_enrollment"] == "" || before.Holder.Family != "instance" || before.Op.Kind != KindMoving || before.Op.Phase == "landing" || before.Op.Phase == "committed" { |
| return Observed{}, ErrWrongState |
| } |
| // Check the instance record even while the claim still says moving: the |
| @@ -79,7 +82,7 @@ func ConvertMovingToRelocating(ctx context.Context, s release.Store, observed Ob |
| if err != nil { |
| return Observed{}, err |
| } |
| - if region == "" || region != fence.binding.Region || before.Op.Intent["source_box"] != fence.record.Box || before.Op.Intent["source_enrollment"] != ref.Enrollment || before.Op.Intent["source_region"] != region || (before.Op.Exec != fence.record.Box && before.Op.Exec != "none" && before.Op.Exec != "") { |
| + if region == "" || region != fence.binding.Region || source != fence.record.Box || before.Op.Intent["source_enrollment"] != ref.Enrollment || (before.Op.Exec != fence.record.Box && before.Op.Exec != "none" && before.Op.Exec != "") { |
| return Observed{}, ErrFence |
| } |
| raw, err := json.Marshal(ref) |
| @@ -112,7 +115,9 @@ func ConvertMovingToRelocating(ctx context.Context, s release.Store, observed Ob |
| c.Op.Journal["source_generation"] = c.Holder.Generation |
| c.Op.Journal["roster_revision"] = strconv.FormatUint(fence.binding.RosterRevision, 10) |
| c.Op.Journal["roster_etag"] = fence.binding.RosterETag |
| - c.Op.Journal["admission_id"] = before.Op.Intent["admission_id"] |
| + if admission := before.Op.Intent["admission_id"]; admission != "" { |
| + c.Op.Journal["admission_id"] = admission |
| + } |
| c.Op.Journal["conversion_claim_gen"] = strconv.FormatUint(before.Gen, 10) |
| c.Op.Journal["conversion_claim_etag"] = observed.ETag |
| c.Op.Kind, c.Op.Phase, c.Op.Exec = KindRelocating, "pinning", "none" |
| @@ -120,16 +125,16 @@ func ConvertMovingToRelocating(ctx context.Context, s release.Store, observed Ob |
| c.Op.Intent = map[string]string{} |
| } |
| c.Op.Intent["region"] = region |
| + c.Op.Intent["source_box"] = source |
| // The original move target is in a different region. It is not a selected |
| // same-region replacement. Keep the move history without authorizing it. |
| - if target := c.Op.Intent["target_box"]; target != "" { |
| - c.Op.Intent["move_target_box"] = target |
| - delete(c.Op.Intent, "target_box") |
| - } |
| - if enrollment := c.Op.Intent["target_enrollment"]; enrollment != "" { |
| - c.Op.Intent["move_target_enrollment"] = enrollment |
| - delete(c.Op.Intent, "target_enrollment") |
| + for from, to := range map[string]string{"target": "move_target_box", "to": "move_to_region", "target_enrollment": "move_target_enrollment"} { |
| + if v := c.Op.Intent[from]; v != "" { |
| + c.Op.Intent[to] = v |
| + } |
| + delete(c.Op.Intent, from) |
| } |
| + delete(c.Op.Intent, "source") |
| participants := make([]string, 0, len(c.Op.Participants)) |
| for _, box := range c.Op.Participants { |
| if box != fence.record.Box { |
diff --git a/internal/hostclaim/relocating_test.go b/internal/hostclaim/relocating_test.go| index 88d2cf2a7..d7a13ab81 100644 |
| --- a/internal/hostclaim/relocating_test.go |
| +++ b/internal/hostclaim/relocating_test.go |
| @@ -34,6 +34,7 @@ func (a c7Authority) ValidateFence(_ context.Context, f termfence.Record) (Fence |
| type c7Guard struct { |
| calls int |
| rejectAt int |
| + region string |
| } |
| func (g *c7Guard) CheckMovingBeforeFlip(_ context.Context, o Observed, id string, ref FenceRef) (string, error) { |
| @@ -44,6 +45,9 @@ func (g *c7Guard) CheckMovingBeforeFlip(_ context.Context, o Observed, id string |
| if g.rejectAt > 0 && g.calls >= g.rejectAt { |
| return "", ErrWrongState |
| } |
| + if g.region != "" { |
| + return g.region, nil |
| + } |
| return "ie", nil |
| } |
| @@ -65,7 +69,8 @@ func c7Fixture(t *testing.T) (*c7Store, Observed, FenceRef, c7Authority) { |
| o, err = Update(ctx, s, o, func(c *Claim) error { |
| c.Holder.Generation, c.Holder.Stamp = "gen-current", 17 |
| c.Op = Op{ID: "move-1", Kind: KindMoving, Phase: "releasing", Exec: "adi-knitting", |
| - Intent: map[string]string{"admission_id": "admission-existing", "source_box": "adi-knitting", "source_enrollment": strings.Repeat("a", 32), "source_region": "ie", "target_box": "bri-knitting", "target_enrollment": strings.Repeat("c", 32)}, |
| + // The intent BeginMove writes, plus the source enrollment the door adds. |
| + Intent: map[string]string{"to": "uk", "source": "adi-knitting", "target": "bri-knitting", "source_enrollment": strings.Repeat("a", 32)}, |
| Journal: map[string]string{"pin": "keep"}, Participants: []string{"adi-knitting", "bri-knitting"}} |
| c.Obligations = []Obligation{{ID: "staging", Op: "move-1", Kind: "park-replica", Region: "uk", Owner: "region:uk", Retirement: "retire-1", Park: &ParkScope{Location: "moves/" + testHost + "/move-1", Incarnation: c.Holder.Incarnation, EvidenceAt: time.Now().UTC(), RequestLimit: 120 * time.Second}}} |
| return nil |
| @@ -95,7 +100,7 @@ func TestC7ConversionPreservesOwnershipAndChores(t *testing.T) { |
| t.Fatal(err) |
| } |
| c := after.Claim |
| - if c.Op.Journal["admission_id"] != "admission-existing" || c.Op.Journal["source_stamp"] != "17" || c.Op.Journal["source_generation"] != "gen-current" || c.Op.Journal["roster_revision"] != "7" || c.Op.Journal["roster_etag"] != "roster-etag" || c.Op.Journal["reason"] != "source-terminated-before-flip" || c.Op.Journal["conversion_claim_etag"] != before.ETag { |
| + if _, ok := c.Op.Journal["admission_id"]; ok || c.Op.Journal["source_stamp"] != "17" || c.Op.Journal["source_generation"] != "gen-current" || c.Op.Journal["roster_revision"] != "7" || c.Op.Journal["roster_etag"] != "roster-etag" || c.Op.Journal["reason"] != "source-terminated-before-flip" || c.Op.Journal["conversion_claim_etag"] != before.ETag { |
| t.Fatalf("missing conversion authority: %+v", c.Op.Journal) |
| } |
| if s.puts != 1 || c.Gen != before.Claim.Gen+1 || guard.calls != 2 { |
| @@ -107,7 +112,7 @@ func TestC7ConversionPreservesOwnershipAndChores(t *testing.T) { |
| if c.Op.ID != "move-1" || c.Op.Kind != KindRelocating || c.Op.Phase != "pinning" || c.Op.Exec != "none" || c.Op.Intent["region"] != "ie" { |
| t.Fatalf("wrong conversion: %+v", c.Op) |
| } |
| - if c.Op.Intent["target_box"] != "" || c.Op.Intent["move_target_box"] != "bri-knitting" || !reflect.DeepEqual(c.Op.Participants, []string{"bri-knitting"}) { |
| + if c.Op.Intent["target_box"] != "" || c.Op.Intent["target"] != "" || c.Op.Intent["move_target_box"] != "bri-knitting" || c.Op.Intent["move_to_region"] != "uk" || c.Op.Intent["source_box"] != "adi-knitting" || !reflect.DeepEqual(c.Op.Participants, []string{"bri-knitting"}) { |
| t.Fatalf("target or barrier: %+v", c.Op) |
| } |
| var saved FenceRef |
| @@ -123,7 +128,7 @@ func TestC7ConversionPreservesOwnershipAndChores(t *testing.T) { |
| } |
| func TestC7ConversionRefusesWithoutWriting(t *testing.T) { |
| - for _, tc := range []string{"landing-record", "flip-during-verify", "landing-claim", "wrong-op", "wrong-source", "wrong-enrollment", "wrong-region", "foreign-exec", "journal-rebind", "authority", "digest", "no-guard", "missing-admission"} { |
| + for _, tc := range []string{"landing-record", "flip-during-verify", "landing-claim", "wrong-op", "wrong-source", "wrong-enrollment", "wrong-region", "foreign-exec", "journal-rebind", "authority", "digest", "no-guard", "missing-enrollment", "missing-source"} { |
| t.Run(tc, func(t *testing.T) { |
| s, o, ref, a := c7Fixture(t) |
| id := "move-1" |
| @@ -138,11 +143,11 @@ func TestC7ConversionRefusesWithoutWriting(t *testing.T) { |
| case "wrong-op": |
| id = "other" |
| case "wrong-source": |
| - o.Claim.Op.Intent["source_box"] = "other" |
| + o.Claim.Op.Intent["source"] = "other" |
| case "wrong-enrollment": |
| o.Claim.Op.Intent["source_enrollment"] = strings.Repeat("d", 32) |
| case "wrong-region": |
| - o.Claim.Op.Intent["source_region"] = "uk" |
| + guard = &c7Guard{region: "uk"} |
| case "foreign-exec": |
| o.Claim.Op.Exec = "target" |
| case "journal-rebind": |
| @@ -153,8 +158,10 @@ func TestC7ConversionRefusesWithoutWriting(t *testing.T) { |
| ref.SHA256 = strings.Repeat("0", 64) |
| case "no-guard": |
| guard = nil |
| - case "missing-admission": |
| - delete(o.Claim.Op.Intent, "admission_id") |
| + case "missing-enrollment": |
| + delete(o.Claim.Op.Intent, "source_enrollment") |
| + case "missing-source": |
| + delete(o.Claim.Op.Intent, "source") |
| } |
| raw, _ := json.Marshal(o.Claim) |
| if _, err := ConvertMovingToRelocating(context.Background(), s, o, id, ref, a, guard); err == nil { |
| @@ -212,7 +219,8 @@ func TestC7RelocatingPhaseValidation(t *testing.T) { |
| _, o, _, _ := c7Fixture(t) |
| c := o.Claim |
| c.Op.Kind, c.Op.Phase = KindRelocating, phase |
| - c.Op.Intent["region"] = "ie" |
| + // A relocating op's own intent (region HA's shape), not a move's. |
| + c.Op.Intent = map[string]string{"region": "ie", "source_box": "adi-knitting", "target_box": "bri-knitting"} |
| switch phase { |
| case "starting", "releasing": |
| c.Op.Exec = "adi-knitting" |
| @@ -249,3 +257,31 @@ func TestC7JournalFenceCannotNameAnotherBox(t *testing.T) { |
| t.Fatal("journal key rebound to another box") |
| } |
| } |
| + |
| +// The C7 contract with moves: a claim BeginMove produced, plus the source |
| +// enrollment the request door records, converts. |
| +func TestC7ConvertsAClaimBeginMoveProduced(t *testing.T) { |
| + s, o, ref, a := c7Fixture(t) |
| + ctx := context.Background() |
| + var err error |
| + o, err = Update(ctx, s, o, func(c *Claim) error { |
| + c.Op = Op{Kind: KindServing, Exec: "adi-knitting"} |
| + c.Obligations = nil |
| + if err := BeginMove("move-1", "member", "adi-knitting", "bri-knitting", "uk", time.Now())(c); err != nil { |
| + return err |
| + } |
| + c.Op.Intent["source_enrollment"] = ref.Enrollment |
| + return nil |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + s.puts = 0 |
| + after, err := ConvertMovingToRelocating(ctx, s, o, "move-1", ref, a, &c7Guard{}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if after.Claim.Op.Kind != KindRelocating || after.Claim.Op.Phase != "pinning" || after.Claim.Op.Intent["source_box"] != "adi-knitting" { |
| + t.Fatalf("not converted: %+v", after.Claim.Op) |
| + } |
| +} |
diff --git a/internal/stopevidence/fds_linux.go b/internal/stopevidence/fds_linux.go| index ce7df03fd..91624fdef 100644 |
| --- a/internal/stopevidence/fds_linux.go |
| +++ b/internal/stopevidence/fds_linux.go |
| @@ -37,6 +37,23 @@ func fdHolders(ctx context.Context, procRoot string, paths []string, ops fdOps) |
| } |
| wanted[filepath.Clean(path)] = true |
| } |
| + // The same file reached by another name (a hard link, another mount of |
| + // the file system) is the same inode: match on device and inode too. |
| + inodes := map[fileID]bool{} |
| + for path := range wanted { |
| + fi, err := ops.stat(path) |
| + if errors.Is(err, os.ErrNotExist) { |
| + continue |
| + } |
| + if err != nil { |
| + return nil, fmt.Errorf("%w: %s: %w", ErrIncomplete, path, err) |
| + } |
| + if id, ok := idOf(fi); ok { |
| + inodes[id] = true |
| + } else { |
| + return nil, fmt.Errorf("%w: %s: no inode", ErrIncomplete, path) |
| + } |
| + } |
| if err := ctx.Err(); err != nil { |
| return nil, err |
| } |
| @@ -84,10 +101,25 @@ func fdHolders(ctx context.Context, procRoot string, paths []string, ops fdOps) |
| if err != nil { |
| return nil, fmt.Errorf("%w: pid %d tid %d fd %s: %w", ErrIncomplete, pid, tid, fd.Name(), err) |
| } |
| - if !filepath.IsAbs(target) { |
| + if filepath.IsAbs(target) && (wanted[filepath.Clean(target)] || wanted[filepath.Clean(strings.TrimSuffix(target, " (deleted)"))]) { |
| + found = true |
| + continue |
| + } |
| + if len(inodes) == 0 || !filepath.IsAbs(target) { |
| + continue // sockets, pipes and anon inodes name no file |
| + } |
| + // Stat through the fd link reaches the open file itself, whatever |
| + // name it was opened by. An unreadable entry fails the whole scan. |
| + fi, err := ops.stat(filepath.Join(fdDir, fd.Name())) |
| + if procGone(err) { |
| continue |
| } |
| - if wanted[filepath.Clean(target)] || wanted[filepath.Clean(strings.TrimSuffix(target, " (deleted)"))] { |
| + if err != nil { |
| + return nil, fmt.Errorf("%w: pid %d tid %d fd %s: %w", ErrIncomplete, pid, tid, fd.Name(), err) |
| + } |
| + if id, ok := idOf(fi); !ok { |
| + return nil, fmt.Errorf("%w: pid %d tid %d fd %s: no inode", ErrIncomplete, pid, tid, fd.Name()) |
| + } else if inodes[id] { |
| found = true |
| } |
| } |
| @@ -122,3 +154,13 @@ func readProcDir(path, owner string, ops fdOps) ([]os.DirEntry, error) { |
| } |
| return entries, err |
| } |
| + |
| +type fileID struct{ dev, ino uint64 } |
| + |
| +func idOf(fi os.FileInfo) (fileID, bool) { |
| + st, ok := fi.Sys().(*syscall.Stat_t) |
| + if !ok { |
| + return fileID{}, false |
| + } |
| + return fileID{dev: uint64(st.Dev), ino: st.Ino}, true |
| +} |
diff --git a/internal/stopevidence/fds_linux_test.go b/internal/stopevidence/fds_linux_test.go| index 9bd1f183e..8a89eafa6 100644 |
| --- a/internal/stopevidence/fds_linux_test.go |
| +++ b/internal/stopevidence/fds_linux_test.go |
| @@ -265,3 +265,48 @@ func TestFDHoldersUnsharedThread(t *testing.T) { |
| t.Fatalf("unshared thread holder: %v %v", got, err) |
| } |
| } |
| + |
| +// A writer holding the database through another name (a hard link, another |
| +// mount of the same file system) holds the same inode; the scan must see it. |
| +func TestFDHoldersMatchesTheInodeNotOnlyThePath(t *testing.T) { |
| + root, dir := t.TempDir(), t.TempDir() |
| + db, alias := filepath.Join(dir, "data.db"), filepath.Join(dir, "alias.db") |
| + if err := os.WriteFile(db, []byte("db"), 0600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Link(db, alias); err != nil { |
| + t.Fatal(err) |
| + } |
| + fd := filepath.Join(root, "40", "task", "40", "fd") |
| + if err := os.MkdirAll(fd, 0700); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(alias, filepath.Join(fd, "0")); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, err := FDHolders(root, []string{db, db + "-wal"}) |
| + if err != nil || !reflect.DeepEqual(got, []int{40}) { |
| + t.Fatalf("hard-linked holder missed: %v %v", got, err) |
| + } |
| + ops := realFDOps() |
| + stat := ops.stat |
| + ops.stat = func(p string) (os.FileInfo, error) { |
| + if p == filepath.Join(fd, "0") { |
| + return nil, syscall.EACCES |
| + } |
| + return stat(p) |
| + } |
| + if got, err := fdHolders(t.Context(), root, []string{db}, ops); !errors.Is(err, ErrIncomplete) || got != nil { |
| + t.Fatalf("unreadable fd counted as not holding: %v %v", got, err) |
| + } |
| + ops = realFDOps() |
| + ops.stat = func(p string) (os.FileInfo, error) { |
| + if p == db { |
| + return nil, syscall.EACCES |
| + } |
| + return stat(p) |
| + } |
| + if got, err := fdHolders(t.Context(), root, []string{db}, ops); !errors.Is(err, ErrIncomplete) || got != nil { |
| + t.Fatalf("unreadable wanted path accepted: %v %v", got, err) |
| + } |
| +} |