amadan

carlos / platform Public

plan: region HA 1a rev 3, answering Astra plan round 2 with dry-run code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 805894fc996acd3c224af8bb3189a0feaef13292 parent e53bb58
2 files changed, +950 −279
  • docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md +909 −279
  • docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-BRIEF-3.md +41 −0

CI passed — run details

diff --git a/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md b/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md
index 34fe9eab8..69a101363 100644
--- a/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md
+++ b/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md
@@ -30,7 +30,7 @@ Spec 1 sits on the host lifecycle claim, which exists only as a spec (rev 6.12 o
- Go test environment on this machine: use the default Go build cache. Do not set `GOCACHE`, `GOMODCACHE` or `GOTMPDIR` under `/tmp`. Scratch files go under `$TMPDIR`. Unix-socket tests need a short `TMPDIR`.
- The branch already contains `origin/main` as of 2026-10-05 (merge 014b0ce1). Before that merge `internal/console`'s tests did not build on this branch; if they fail to build again, merge main first.
- Local test runs need `GOFLAGS=-buildvcs=false` (some console tests build a binary and fail on VCS stamping in a worktree) and, inside the Claude sandbox, `GOCACHE=$TMPDIR/gocache`. Tests that open Unix sockets (`internal/activator`, `internal/edge`) fail inside the sandbox with `socket: operation not permitted`: that is the sandbox, not the change. Run those packages with the sandbox off, or rely on CI.
-- This plan's code was dry-run on 2026-10-05: every new package and both patch tasks were built, cross-built for darwin and windows, and tested, then removed. Two things the dry run could not do are called out where they matter: the six real-inotify tests (Task 7, step 10) and the IMDS signature check (Task 1, step 7).
+- This plan's code was dry-run on 2026-10-05: every new package and both patch tasks were built, cross-built for darwin and windows, and tested, then removed. Two things the dry run could not do are called out where they matter: the real-inotify tests (Task 7, step 10) and the IMDS signature check (Task 1, step 7).
- Gate before each commit: `go build ./... && go vet ./... && gofmt -l . && go test ./internal/<pkg>/... -count=1`. Full gate before the branch lands: `make test`.
- Exact values from the spec: continuous deadline **1 second** from the first unpublished modification; repair scan **10 minutes**; polling fallback **1 second**; snapshot interval default **15 minutes**; `files_durability` default **`snapshot`** (H8); capability string **`region-ha-1`**; termination fence accepts **only** state `terminated`.
@@ -1504,7 +1504,7 @@ Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>"
- Test: `internal/edge/homemap_test.go`, `internal/host/edgesfile_test.go`
**Interfaces:**
-- Produces: `edgeHost.Stamp int64 \`json:"stamp,omitempty"\`` (writer), `edgesHostView.Stamp int64 \`json:"stamp,omitempty"\`` (reader), `homeEntry.stamp int64`, and a changed signature: `foldHomes(files map[string]edgesFileView, self string) (map[string]homeEntry, []string)`, where the second result is the sorted list of hosts two boxes claim at the same positive stamp. Its one caller is `(*homeMap).Refresh`.
+- Produces: `edgeHost.Stamp int64 \`json:"stamp,omitempty"\`` (writer), `edgesHostView.Stamp int64 \`json:"stamp,omitempty"\`` (reader), `homeEntry.stamp int64`, and a changed signature: `foldHomes(files map[string]edgesFileView, self string) (map[string]homeEntry, []string)`, where the second result is the sorted list of hosts that two boxes publish as home at the same positive stamp, found over every file including this box's own (`tiedStamps`). Its one caller is `(*homeMap).Refresh`.
- Nothing sets `Stamp` yet: plan 1b sets it from the claim's `holder.stamp`. Until then every entry is unstamped and today's rule applies unchanged. That is what makes this task safe to land alone.
The rule (spec HA4, §5.4), applied when two files claim one host as a dial target:
@@ -1587,12 +1587,16 @@ func TestHomeMapAStampedEntryWithNoDialAddressDoesNotWin(t *testing.T) {
}
}
+func file(box, eip string, hosts map[string]edgesHostView) edgesFileView {
+ return edgesFileView{V: 1, Box: box, EIP: eip, Hosts: hosts}
+}
+
func TestHomeMapReportsTwoBoxesAtTheSameStamp(t *testing.T) {
files := map[string]edgesFileView{
- "control/edges/annie.json": {V: 1, Box: "annie", EIP: "1.1.1.1", Hosts: map[string]edgesHostView{
- "tied.test": {Role: "home", Stamp: 7}, "fine.test": {Role: "home", Stamp: 7}, "plain.test": {Role: "home"}}},
- "control/edges/betty.json": {V: 1, Box: "betty", EIP: "2.2.2.2", Hosts: map[string]edgesHostView{
- "tied.test": {Role: "home", Stamp: 7}, "fine.test": {Role: "home", Stamp: 9}, "plain.test": {Role: "home"}}},
+ "control/edges/annie.json": file("annie", "1.1.1.1", map[string]edgesHostView{
+ "tied.test": {Role: "home", Stamp: 7}, "fine.test": {Role: "home", Stamp: 7}, "plain.test": {Role: "home"}}),
+ "control/edges/betty.json": file("betty", "2.2.2.2", map[string]edgesHostView{
+ "tied.test": {Role: "home", Stamp: 7}, "fine.test": {Role: "home", Stamp: 9}, "plain.test": {Role: "home"}}),
}
hosts, tied := foldHomes(files, "carol")
if len(tied) != 1 || tied[0] != "tied.test" {
@@ -1605,6 +1609,39 @@ func TestHomeMapReportsTwoBoxesAtTheSameStamp(t *testing.T) {
t.Fatalf("fine.test box = %q", hosts["fine.test"].box)
}
}
+
+// A tie between two boxes is reported even when a third box holds a higher
+// stamp and wins the dial: the report does not depend on the winner.
+func TestHomeMapReportsATieBelowTheWinner(t *testing.T) {
+ files := map[string]edgesFileView{
+ "control/edges/annie.json": file("annie", "1.1.1.1", map[string]edgesHostView{"h.test": {Role: "home", Stamp: 9}}),
+ "control/edges/betty.json": file("betty", "2.2.2.2", map[string]edgesHostView{"h.test": {Role: "home", Stamp: 7}}),
+ "control/edges/carol.json": file("carol", "3.3.3.3", map[string]edgesHostView{"h.test": {Role: "home", Stamp: 7}}),
+ }
+ hosts, tied := foldHomes(files, "dave")
+ if len(tied) != 1 || tied[0] != "h.test" {
+ t.Fatalf("tied = %v, want [h.test]", tied)
+ }
+ if hosts["h.test"].box != "annie" {
+ t.Fatalf("the highest stamp still wins the dial; got %q", hosts["h.test"].box)
+ }
+}
+
+// In a two-box region each box excludes itself from routing. It must
+// still see that it and its neighbour publish one host at one stamp.
+func TestHomeMapReportsATieWithThisBoxItself(t *testing.T) {
+ files := map[string]edgesFileView{
+ "control/edges/annie.json": file("annie", "1.1.1.1", map[string]edgesHostView{"h.test": {Role: "home", Stamp: 7}}),
+ "control/edges/betty.json": file("betty", "2.2.2.2", map[string]edgesHostView{"h.test": {Role: "home", Stamp: 7}}),
+ }
+ hosts, tied := foldHomes(files, "annie")
+ if len(tied) != 1 || tied[0] != "h.test" {
+ t.Fatalf("tied = %v, want [h.test]", tied)
+ }
+ if hosts["h.test"].box != "betty" {
+ t.Fatalf("this box is still excluded from its own routing; got %q", hosts["h.test"].box)
+ }
+}
```
Append to `internal/host/edgesfile_test.go`:
@@ -1661,13 +1698,6 @@ and in `homeEntry` after `account, app`:
func foldHomes(files map[string]edgesFileView, self string) (map[string]homeEntry, []string) {
```
-and, where it declares `hosts`, declare the set of ties beside it:
-
-```go
- hosts := make(map[string]homeEntry)
- tiedSet := map[string]bool{}
-```
-
Set the stamp on the incoming entry. Change:
```go
@@ -1694,24 +1724,55 @@ Replace the merge condition:
with:
```go
- if in.dial && prev.dial && in.alias == prev.alias && in.stamp != 0 && in.stamp == prev.stamp && in.box != prev.box {
- tiedSet[host] = true
- }
if dialBeats(in, prev) {
merged.eip, merged.box, merged.alias, merged.dial = in.eip, in.box, in.alias, true
merged.account, merged.app, merged.stamp = in.account, in.app, in.stamp
}
```
-Replace the function's final `return hosts` with:
+Replace the function's final `return hosts` with `return hosts, tiedStamps(files)`, and add below it:
```go
- tied := make([]string, 0, len(tiedSet))
- for h := range tiedSet {
- tied = append(tied, h)
+// tiedStamps lists, sorted, the hosts that two boxes publish as home at
+// the same positive stamp. It looks at every file, this box's own
+// included, and at every pair, not only at whichever entry won the dial:
+// in a two-box region each box must be able to see that it and its
+// neighbour disagree, and a third box's higher stamp must not hide a tie
+// between two others.
+//
+// Unstamped duplicates are not reported. They are normal: a static site
+// or a public address route runs on every box, and every box publishes it.
+func tiedStamps(files map[string]edgesFileView) []string {
+ type key struct {
+ host string
+ stamp int64
+ }
+ boxes := map[key]map[string]bool{}
+ for _, f := range files {
+ if f.Box == "" {
+ continue
+ }
+ for host, h := range f.Hosts {
+ if host == "" || h.Role != edgeRoleHome || h.Stamp <= 0 {
+ continue
+ }
+ k := key{host, h.Stamp}
+ if boxes[k] == nil {
+ boxes[k] = map[string]bool{}
+ }
+ boxes[k][f.Box] = true
+ }
+ }
+ seen := map[string]bool{}
+ var tied []string
+ for k, set := range boxes {
+ if len(set) > 1 && !seen[k.host] {
+ seen[k.host] = true
+ tied = append(tied, k.host)
+ }
}
slices.Sort(tied)
- return hosts, tied
+ return tied
}
// dialBeats reports whether a later file's entry takes the dial from the
@@ -1740,6 +1801,8 @@ func dialBeats(in, prev homeEntry) bool {
}
```
+Ties are found by `tiedStamps` in a pass of its own, over every file, rather than inside the fold. The fold skips this box's own file and compares each entry only with the current winner, so it cannot see a tie that involves this box, or one between two boxes that both lose to a third.
+
In `(*homeMap).Refresh`, replace the one call:
```go
@@ -1753,8 +1816,8 @@ with:
if len(tied) > 0 {
// One claim revision names one executor, so two boxes publishing
// one host at the same stamp is a fault upstream. The fold stays
- // deterministic (first box in sorted order); say so once a refresh.
- log.Printf("edge: home map: %d host(s) claimed by two boxes at the same installation stamp, e.g. %s: keeping the first box in sorted order", len(tied), tied[0])
+ // deterministic; say so once a refresh.
+ log.Printf("edge: home map: %d host(s) published by two boxes at the same installation stamp, e.g. %s", len(tied), tied[0])
}
snap := &homeSnapshot{files: next, hosts: hosts}
```
@@ -1838,6 +1901,7 @@ import (
"errors"
"os"
"path/filepath"
+ "reflect"
"strings"
"testing"
@@ -2198,6 +2262,51 @@ func TestRestoreIntoAMissingDirectoryIsAnError(t *testing.T) {
t.Fatal("restored into a directory that does not exist")
}
}
+
+// Plan round 2, finding 3: a restore that returns nil has made its names
+// durable, and one that could not must say so.
+func TestRestoreSyncsEveryDirectoryItChangedBeforeReturning(t *testing.T) {
+ ctx, s := context.Background(), release.NewDirStore(t.TempDir())
+ p := open(t, s)
+ stage(t, p, "top.txt", "t")
+ stage(t, p, "a/b/deep.txt", "d")
+ stage(t, p, "gone/old.txt", "o")
+ commit(t, p)
+ dir := t.TempDir()
+ if _, _, err := Restore(ctx, s, pfx, dir); err != nil {
+ t.Fatal(err)
+ }
+ if err := p.StageDelete("gone/old.txt"); err != nil {
+ t.Fatal(err)
+ }
+ commit(t, p)
+
+ var synced []string
+ orig := syncDir
+ t.Cleanup(func() { syncDir = orig })
+ syncDir = func(root *os.Root, rel string) error {
+ // Everything must already be in place when the first sync runs.
+ if len(synced) == 0 {
+ if _, err := os.Stat(filepath.Join(dir, "gone", "old.txt")); !os.IsNotExist(err) {
+ t.Error("a directory was synced before the tombstone was applied")
+ }
+ }
+ synced = append(synced, rel)
+ return orig(root, rel)
+ }
+ if _, _, err := Restore(ctx, s, pfx, dir); err != nil {
+ t.Fatal(err)
+ }
+ want := []string{"a/b", "a", "gone", "."}
+ if !reflect.DeepEqual(synced, want) {
+ t.Fatalf("synced %v, want %v (deepest first, the root last)", synced, want)
+ }
+
+ syncDir = func(*os.Root, string) error { return errors.New("disk says no") }
+ if _, _, err := Restore(ctx, s, pfx, dir); err == nil || !strings.Contains(err.Error(), "disk says no") {
+ t.Fatalf("err = %v; a failed directory sync must fail the restore", err)
+ }
+}
```
- [ ] **Step 2: Run them to see them fail**
@@ -2521,6 +2630,16 @@ import (
"github.com/carlosframework/platform/internal/release"
)
+// syncDir makes a directory's entries durable. A test seam.
+var syncDir = func(root *os.Root, rel string) error {
+ d, err := root.Open(filepath.FromSlash(rel))
+ if err != nil {
+ return err
+ }
+ defer d.Close()
+ return d.Sync()
+}
+
type RestoreStats struct {
Written, Removed int
Skipped []string // paths the manifest holds as present but unpublished
@@ -2535,6 +2654,11 @@ type RestoreStats struct {
// one bad path changes nothing. Every file operation goes through an
// os.Root opened on dir, so nothing is written or removed outside it even
// if dir already holds a symlink pointing elsewhere.
+//
+// When Restore returns nil, what it did is durable: each file was synced
+// before its rename, and every directory in which a name was created,
+// replaced or removed has been synced since. A caller may record the
+// installation as complete on that basis, and on no weaker one.
func Restore(ctx context.Context, s release.Store, prefix, dir string) (Manifest, RestoreStats, error) {
var st RestoreStats
m, _, _, err := load(ctx, s, prefix)
@@ -2563,6 +2687,7 @@ func Restore(ctx context.Context, s release.Store, prefix, dir string) (Manifest
// Tombstones first, deepest first, so a directory that became a file
// is emptied before the file is written over it.
+ touched := map[string]bool{} // directories whose entries changed
sort.Slice(dead, func(i, j int) bool {
di, dj := strings.Count(dead[i], "/"), strings.Count(dead[j], "/")
if di != dj {
@@ -2577,6 +2702,7 @@ func Restore(ctx context.Context, s release.Store, prefix, dir string) (Manifest
}
if removed {
st.Removed++
+ touched[path.Dir(p)] = true
}
}
sort.Strings(live)
@@ -2594,6 +2720,39 @@ func Restore(ctx context.Context, s release.Store, prefix, dir string) (Manifest
return Manifest{}, st, fmt.Errorf("hostfiles: write %s: %w", p, err)
}
st.Written++
+ // The file's own directory, and every directory above it up to
+ // the root: any of them may have been created, or had something
+ // in the way removed, on the way to this file.
+ for d := path.Dir(p); ; d = path.Dir(d) {
+ touched[d] = true
+ if d == "." {
+ break
+ }
+ }
+ }
+ // Deepest first, so a directory is durable before the entry that
+ // names it is.
+ dirs := make([]string, 0, len(touched))
+ for d := range touched {
+ dirs = append(dirs, d)
+ }
+ sort.Slice(dirs, func(i, j int) bool {
+ di, dj := strings.Count(dirs[i], "/"), strings.Count(dirs[j], "/")
+ if (dirs[i] == ".") != (dirs[j] == ".") {
+ return dirs[j] == "."
+ }
+ if di != dj {
+ return di > dj
+ }
+ return dirs[i] < dirs[j]
+ })
+ for _, d := range dirs {
+ if _, err := root.Lstat(filepath.FromSlash(d)); errors.Is(err, fs.ErrNotExist) {
+ continue // emptied by a tombstone and then replaced by a file
+ }
+ if err := syncDir(root, d); err != nil {
+ return Manifest{}, st, fmt.Errorf("hostfiles: sync directory %s: %w", d, err)
+ }
}
return m, st, nil
}
@@ -2750,6 +2909,8 @@ Expected: PASS.
`Restore` does every file operation through an `os.Root` opened on the destination (Go 1.24+), so a symlink already sitting in the destination cannot carry a write or a delete outside it. The destination must exist.
+**A restore that returns nil is durable.** Each file is synced before its rename, and before returning `Restore` syncs every directory in which a name was created, replaced or removed, deepest first, and fails if any of those syncs fails. Plan 1b writes the installation-complete marker (Task 10) only after `Restore` returns nil; without the directory syncs that marker could outlive the names it vouches for across a crash. `TestRestoreSyncsEveryDirectoryItChangedBeforeReturning` pins the set, the order and the failure.
+
- [ ] **Step 7: Commit**
```bash
@@ -2764,8 +2925,8 @@ Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>"
### Task 6: Publishing a whole directory (`hostfiles.SyncDir`)
**Files:**
-- Create: `internal/hostfiles/sync.go`, `internal/hostfiles/open_unix.go`, `internal/hostfiles/open_other.go`
-- Test: `internal/hostfiles/sync_test.go`, `internal/hostfiles/sync_unix_test.go`
+- Create: `internal/hostfiles/sync.go`, `internal/hostfiles/dir_unix.go`, `internal/hostfiles/walk_unix.go`, `internal/hostfiles/dir_other.go`
+- Test: `internal/hostfiles/sync_test.go`, `internal/hostfiles/sync_unix_test.go` (both `//go:build unix`)
**Interfaces:**
- Consumes: Task 5's `Publisher`.
@@ -2780,19 +2941,17 @@ Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>"
// on disk. It does not commit. skip, when non-nil, excludes a relative
// path (and, for a directory, everything under it).
func SyncDir(ctx context.Context, p *Publisher, dir string, skip func(rel string, isDir bool) bool) (SyncStats, error)
- // readStable reads one file, named relative to an opened root, for publishing.
- func readStable(root *os.Root, rel string) (body []byte, mode uint32, state fileState, err error)
- func syncRoot(ctx context.Context, p *Publisher, root *os.Root, skip func(rel string, isDir bool) bool) (SyncStats, error)
+ // A directory held open by descriptor; nothing beneath it is reached through a symlink.
+ type dirHandle struct{ /* unexported */ }
+ func openTree(path string) (*dirHandle, error)
+ func (d *dirHandle) descend(rel string) (sub *dirHandle, gone bool, err error)
+ func (d *dirHandle) Close() error
+ // readStable reads the file name inside d for publishing.
+ func readStable(d *dirHandle, name string) (body []byte, mode uint32, state fileState, err error)
+ func syncRoot(ctx context.Context, p *Publisher, root *dirHandle, skip func(rel string, isDir bool) bool) (SyncStats, error)
func skipped(skip func(string, bool) bool, rel string, isDir bool) bool // rel or any directory above it is excluded
- const openNonblock // O_NONBLOCK on unix, 0 elsewhere
- type fileState int
- const (
- fileOK fileState = iota
- fileTorn // changed while it was read; body is the image read
- fileGone
- fileNotRegular
- fileTooLarge
- )
+ // fileState gains: fileUnreadable (exists, could not be read) and fileRetry (replaced mid-look).
+ // SyncStats gains: Unreadable []string.
```
This one function serves three callers in plan 1b: the `snapshot` setting (run every `files_snapshot_interval`), the continuous publisher's ten-minute repair scan, and the release sequence's "publish every pending mutation" step. That is why this plan has no separate sealed-archive snapshot: a periodic full `SyncDir` gives the `snapshot` setting the same result through the same manifest (recorded as a spec note in Task 11).
@@ -2802,6 +2961,8 @@ This one function serves three callers in plan 1b: the `snapshot` setting (run e
`internal/hostfiles/sync_test.go`:
```go
+//go:build unix
+
package hostfiles
import (
@@ -2992,7 +3153,7 @@ func TestSyncDirDoesNotTombstoneWhatASkippedDirectoryHolds(t *testing.T) {
}
```
-Then `internal/hostfiles/sync_unix_test.go`, for the cases that need FIFOs and symlinks. Two of them use the seam `betweenLookAndOpen` to swap a file after `readStable` has inspected it and before it opens it, which makes the race deterministic:
+Then `internal/hostfiles/sync_unix_test.go`, for special files, substitution races and unreadable files. `swapAt` uses the seam `betweenLookAndOpen` to swap a file or directory in the gap between its inspection and its open, which makes each race deterministic. Two tests skip where they cannot run: the socket test inside a sandbox that forbids sockets, the device test without root.
```go
//go:build unix
@@ -3001,6 +3162,7 @@ package hostfiles
import (
"context"
+ "net"
"os"
"path/filepath"
"reflect"
@@ -3011,26 +3173,115 @@ import (
"github.com/carlosframework/platform/internal/release"
)
-// Review Focus 2: special files are skipped, never opened.
+// within fails the test if fn has not returned in two seconds: the way a
+// blocked open shows up.
+func within(t *testing.T, what string, fn func()) {
+ t.Helper()
+ done := make(chan struct{})
+ go func() { defer close(done); fn() }()
+ select {
+ case <-done:
+ case <-time.After(2 * time.Second):
+ t.Fatalf("%s did not return: something blocked", what)
+ }
+}
+
+// swapAt arranges for fn to run once, at the nth pause between a look and
+// an open (1 is the first).
+func swapAt(t *testing.T, n int, fn func()) {
+ t.Helper()
+ orig, calls := betweenLookAndOpen, 0
+ t.Cleanup(func() { betweenLookAndOpen = orig })
+ betweenLookAndOpen = func() {
+ calls++
+ if calls == n {
+ fn()
+ }
+ }
+}
+
+func tree(t *testing.T, dir string) *dirHandle {
+ t.Helper()
+ d, err := openTree(dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { d.Close() })
+ return d
+}
+
+func fifo(t *testing.T, path string) {
+ t.Helper()
+ if err := syscall.Mkfifo(path, 0o600); err != nil {
+ t.Skipf("no FIFOs here: %v", err)
+ }
+}
+
+// Review Focus 2: special files are skipped, never opened for reading.
func TestSyncDirSkipsAFIFO(t *testing.T) {
ctx, s, src := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir()
write(t, src, "real.txt", "r")
- if err := syscall.Mkfifo(filepath.Join(src, "pipe"), 0o600); err != nil {
- t.Skipf("no FIFOs here: %v", err)
+ fifo(t, filepath.Join(src, "pipe"))
+ p := open(t, s)
+ var st SyncStats
+ within(t, "SyncDir", func() { st, _ = SyncDir(ctx, p, src, nil) })
+ if st.Published != 1 || !reflect.DeepEqual(st.NotRegular, []string{"pipe"}) {
+ t.Fatalf("stats = %+v", st)
+ }
+}
+
+func TestSyncDirSkipsASocket(t *testing.T) {
+ ctx, s, src := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir()
+ l, err := net.Listen("unix", filepath.Join(src, "sock"))
+ if err != nil {
+ t.Skipf("no Unix sockets here: %v", err)
}
+ defer l.Close()
p := open(t, s)
- done := make(chan SyncStats, 1)
- go func() {
- st, _ := SyncDir(ctx, p, src, nil)
- done <- st
- }()
- select {
- case st := <-done:
- if st.Published != 1 || !reflect.DeepEqual(st.NotRegular, []string{"pipe"}) {
- t.Fatalf("stats = %+v", st)
+ var st SyncStats
+ within(t, "SyncDir", func() { st, _ = SyncDir(ctx, p, src, nil) })
+ if !reflect.DeepEqual(st.NotRegular, []string{"sock"}) || len(p.Paths()) != 0 {
+ t.Fatalf("stats = %+v, paths = %v", st, p.Paths())
+ }
+}
+
+func TestSyncDirSkipsADevice(t *testing.T) {
+ if os.Getuid() != 0 {
+ t.Skip("making a device node needs root")
+ }
+ ctx, s, src := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir()
+ if err := syscall.Mknod(filepath.Join(src, "null"), syscall.S_IFCHR|0o600, 1<<8|3); err != nil {
+ t.Skipf("mknod: %v", err)
+ }
+ p := open(t, s)
+ st, err := SyncDir(ctx, p, src, nil)
+ if err != nil || !reflect.DeepEqual(st.NotRegular, []string{"null"}) {
+ t.Fatalf("stats = %+v, err = %v", st, err)
+ }
+}
+
+// A symlinked directory is never descended into, wherever it points.
+func TestSyncDirDoesNotDescendASymlinkedDirectory(t *testing.T) {
+ ctx, s, src, outside := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir(), t.TempDir()
+ if err := os.WriteFile(filepath.Join(outside, "secret"), []byte("s"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ write(t, src, "inside/own", "o")
+ for link, target := range map[string]string{"out": outside, "in": "inside"} {
+ if err := os.Symlink(target, filepath.Join(src, link)); err != nil {
+ t.Fatal(err)
}
- case <-time.After(2 * time.Second):
- t.Fatal("SyncDir blocked on a FIFO")
+ }
+ p := open(t, s)
+ st, err := SyncDir(ctx, p, src, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got := p.Paths(); !reflect.DeepEqual(got, []string{"inside/own"}) {
+ t.Fatalf("Paths = %v", got)
+ }
+ if !reflect.DeepEqual(st.NotRegular, []string{"in", "out"}) {
+ t.Fatalf("NotRegular = %v", st.NotRegular)
}
}
@@ -3039,14 +3290,8 @@ func TestSyncDirSkipsAFIFO(t *testing.T) {
func TestReadStableDoesNotBlockOnAFIFOSwappedIn(t *testing.T) {
dir := t.TempDir()
write(t, dir, "f", "regular")
- root, err := os.OpenRoot(dir)
- if err != nil {
- t.Fatal(err)
- }
- defer root.Close()
- orig := betweenLookAndOpen
- t.Cleanup(func() { betweenLookAndOpen = orig })
- betweenLookAndOpen = func() {
+ d := tree(t, dir)
+ swapAt(t, 1, func() {
full := filepath.Join(dir, "f")
if err := os.Remove(full); err != nil {
t.Error(err)
@@ -3054,73 +3299,143 @@ func TestReadStableDoesNotBlockOnAFIFOSwappedIn(t *testing.T) {
if err := syscall.Mkfifo(full, 0o600); err != nil {
t.Error(err)
}
- }
- type result struct {
- body []byte
- state fileState
- }
- done := make(chan result, 1)
- go func() {
- body, _, state, _ := readStable(root, "f")
- done <- result{body, state}
- }()
- select {
- case r := <-done:
- if r.state != fileNotRegular || r.body != nil {
- t.Fatalf("state = %v, %d bytes; want fileNotRegular and nothing read", r.state, len(r.body))
- }
- case <-time.After(2 * time.Second):
- t.Fatal("readStable blocked opening a FIFO")
+ })
+ var body []byte
+ var state fileState
+ within(t, "readStable", func() { body, _, state, _ = readStable(d, "f") })
+ if state != fileNotRegular || body != nil {
+ t.Fatalf("state = %v, %d bytes; want fileNotRegular and nothing read", state, len(body))
}
}
-// A parent directory swapped for a symlink to somewhere else, after the
-// file was inspected, must not let the read leave the root.
-func TestReadStableDoesNotFollowAnAncestorSwappedForASymlink(t *testing.T) {
- dir, outside := t.TempDir(), t.TempDir()
- write(t, dir, "sub/f", "inside")
- if err := os.WriteFile(filepath.Join(outside, "f"), []byte("outside secret"), 0o600); err != nil {
- t.Fatal(err)
- }
- root, err := os.OpenRoot(dir)
- if err != nil {
- t.Fatal(err)
- }
- defer root.Close()
- orig := betweenLookAndOpen
- t.Cleanup(func() { betweenLookAndOpen = orig })
- betweenLookAndOpen = func() {
- if err := os.RemoveAll(filepath.Join(dir, "sub")); err != nil {
+// A directory swapped for a FIFO between being listed and being entered
+// must not hang the scan.
+func TestSyncDirDoesNotBlockOnADirectorySwappedForAFIFO(t *testing.T) {
+ ctx, s, src := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir()
+ write(t, src, "d/child", "c")
+ write(t, src, "z.txt", "z")
+ swapAt(t, 1, func() { // the first pause is before entering "d"
+ if err := os.RemoveAll(filepath.Join(src, "d")); err != nil {
t.Error(err)
}
- if err := os.Symlink(outside, filepath.Join(dir, "sub")); err != nil {
+ if err := syscall.Mkfifo(filepath.Join(src, "d"), 0o600); err != nil {
t.Error(err)
}
- }
- body, _, state, err := readStable(root, "sub/f")
+ })
+ p := open(t, s)
+ var st SyncStats
+ var err error
+ within(t, "SyncDir", func() { st, err = SyncDir(ctx, p, src, nil) })
if err != nil {
t.Fatal(err)
}
- if state != fileNotRegular || body != nil {
- t.Fatalf("state = %v, body = %q; the outside file must not be read", state, body)
+ if !reflect.DeepEqual(st.NotRegular, []string{"d"}) || !reflect.DeepEqual(p.Paths(), []string{"z.txt"}) {
+ t.Fatalf("stats = %+v, paths = %v", st, p.Paths())
}
}
-// A symlinked directory present from the start is never descended into.
-func TestSyncDirDoesNotDescendASymlinkedDirectory(t *testing.T) {
- ctx, s, src, outside := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir(), t.TempDir()
- if err := os.WriteFile(filepath.Join(outside, "secret"), []byte("s"), 0o600); err != nil {
+// A parent directory swapped for a symlink, to outside or to an excluded
+// directory inside, must not let a queued path be read through it.
+func TestPublishOneNeverReadsThroughASwappedAncestor(t *testing.T) {
+ for name, target := range map[string]string{"a link to outside": "OUTSIDE", "a link to an excluded directory inside": "cache"} {
+ t.Run(name, func(t *testing.T) {
+ ctx, s, src, outside := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir(), t.TempDir()
+ write(t, src, "sub/x", "ours")
+ write(t, src, "cache/x", "excluded")
+ if err := os.WriteFile(filepath.Join(outside, "x"), []byte("outside secret"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ skip := func(rel string, isDir bool) bool { return isDir && rel == "cache" }
+ p := open(t, s)
+ if _, err := SyncDir(ctx, p, src, skip); err != nil {
+ t.Fatal(err)
+ }
+ commit(t, p)
+ if target == "OUTSIDE" {
+ target = outside
+ }
+ swapAt(t, 1, func() { // before "sub" is opened
+ if err := os.RemoveAll(filepath.Join(src, "sub")); err != nil {
+ t.Error(err)
+ }
+ if err := os.Symlink(target, filepath.Join(src, "sub")); err != nil {
+ t.Error(err)
+ }
+ })
+ if _, err := publishOne(ctx, p, tree(t, src), "sub/x", skip); err != nil {
+ t.Fatal(err)
+ }
+ commit(t, p)
+ m, _, _ := Load(ctx, s, pfx)
+ if e := m.Entries["sub/x"]; !e.Deleted {
+ t.Fatalf("sub/x = %+v; the directory is gone, so the file is: want a tombstone, and no content read through the link", e)
+ }
+ })
+ }
+}
+
+// Finding 2 of plan round 2: a file that still exists is never tombstoned
+// just because it could not be read.
+func TestAnUnreadableFileKeepsItsPublishedEntry(t *testing.T) {
+ if os.Getuid() == 0 {
+ t.Skip("root can read anything")
+ }
+ ctx, s, src := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir()
+ write(t, src, "locked", "v1")
+ p := open(t, s)
+ if _, err := SyncDir(ctx, p, src, nil); err != nil {
t.Fatal(err)
}
- if err := os.Symlink(outside, filepath.Join(src, "linkdir")); err != nil {
+ commit(t, p)
+ if err := os.Chmod(filepath.Join(src, "locked"), 0); err != nil {
t.Fatal(err)
}
+ st, err := SyncDir(ctx, p, src, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if st.Deleted != 0 || p.Dirty() || !reflect.DeepEqual(st.Unreadable, []string{"locked"}) {
+ t.Fatalf("stats = %+v dirty = %v", st, p.Dirty())
+ }
+ if _, err := publishOne(ctx, p, tree(t, src), "locked", nil); err == nil {
+ t.Fatal("publishOne reported success for a file it could not read")
+ }
+ if p.Dirty() {
+ t.Fatal("publishOne staged a change for a file it could not read")
+ }
+}
+
+// A regular file replaced by another regular file mid-look is read again
+// later, not tombstoned and not read as if it were the first.
+func TestARegularFileReplacedMidLookIsRetriedNotDeleted(t *testing.T) {
+ ctx, s, src := context.Background(), release.NewDirStore(t.TempDir()), t.TempDir()
+ write(t, src, "f", "v1")
p := open(t, s)
if _, err := SyncDir(ctx, p, src, nil); err != nil {
t.Fatal(err)
}
- if got := p.Paths(); len(got) != 0 {
- t.Fatalf("Paths = %v", got)
+ commit(t, p)
+ replace := func() {
+ tmp := filepath.Join(src, "f.new")
+ if err := os.WriteFile(tmp, []byte("v2"), 0o640); err != nil {
+ t.Error(err)
+ }
+ if err := os.Rename(tmp, filepath.Join(src, "f")); err != nil {
+ t.Error(err)
+ }
+ }
+ swapAt(t, 1, replace)
+ again, err := publishOne(ctx, p, tree(t, src), "f", nil)
+ if err != nil || !again || p.Dirty() {
+ t.Fatalf("again = %v, err = %v, dirty = %v; want a retry and nothing staged", again, err, p.Dirty())
+ }
+ again, err = publishOne(ctx, p, tree(t, src), "f", nil)
+ if err != nil || again {
+ t.Fatalf("second look: again = %v, err = %v", again, err)
+ }
+ commit(t, p)
+ if !manifestHas(s, "f", sum([]byte("v2"))) {
+ t.Fatal("the replacement was not published")
}
}
```
@@ -3137,13 +3452,9 @@ package hostfiles
import (
"context"
- "errors"
"fmt"
"io"
- "io/fs"
- "os"
"path"
- "path/filepath"
"sort"
)
@@ -3155,81 +3466,20 @@ const (
fileGone
fileNotRegular
fileTooLarge
+ fileUnreadable
+ fileRetry
)
-// Test seams. betweenLookAndOpen runs after a file has been inspected and
-// before it is opened; readAll is the one place file contents are read.
+// Test seams. betweenLookAndOpen runs after something has been inspected
+// and before it is opened; readAll is the one place file contents are read.
var (
betweenLookAndOpen = func() {}
readAll = io.ReadAll
)
-// readStable reads one file, named relative to root, for publishing.
-//
-// It opens through an os.Root, so a path cannot leave the directory even
-// if one of its ancestors is swapped for a symlink. It opens without
-// blocking, so a FIFO swapped in for the file cannot hang the publisher.
-// It reads only if what it opened is the regular file it inspected, and a
-// file over MaxFileBytes is refused on its size alone, before any read.
-//
-// If the file's size, mtime or identity changed between the first look
-// and the last byte, the bytes read come back as fileTorn: a possibly
-// inconsistent image the caller publishes flagged and reads again later.
-func readStable(root *os.Root, rel string) ([]byte, uint32, fileState, error) {
- name := filepath.FromSlash(rel)
- before, err := root.Lstat(name)
- if errors.Is(err, fs.ErrNotExist) {
- return nil, 0, fileGone, nil
- }
- if err != nil {
- // Includes a path that escapes root through a symlinked ancestor.
- return nil, 0, fileNotRegular, nil
- }
- if !before.Mode().IsRegular() {
- return nil, 0, fileNotRegular, nil
- }
- if before.Size() > MaxFileBytes {
- return nil, 0, fileTooLarge, nil
- }
- betweenLookAndOpen()
- f, err := root.OpenFile(name, os.O_RDONLY|openNonblock, 0)
- if errors.Is(err, fs.ErrNotExist) {
- return nil, 0, fileGone, nil
- }
- if err != nil {
- return nil, 0, fileNotRegular, nil
- }
- defer f.Close()
- opened, err := f.Stat()
- if err != nil {
- return nil, 0, fileOK, err
- }
- if !opened.Mode().IsRegular() || !os.SameFile(before, opened) {
- // Replaced between the look and the open. Whatever it is now, it
- // is not the file that was inspected: do not read it.
- return nil, 0, fileNotRegular, nil
- }
- if opened.Size() > MaxFileBytes {
- return nil, 0, fileTooLarge, nil
- }
- body, err := readAll(io.LimitReader(f, MaxFileBytes+1))
- if err != nil {
- return nil, 0, fileOK, err
- }
- if int64(len(body)) > MaxFileBytes {
- return nil, 0, fileTooLarge, nil // it grew past the cap while being read
- }
- state := fileOK
- after, err := root.Lstat(name)
- if err != nil || !os.SameFile(before, after) || after.Size() != before.Size() || !after.ModTime().Equal(before.ModTime()) {
- state = fileTorn
- }
- return body, uint32(before.Mode().Perm()), state, nil
-}
-
type SyncStats struct {
- Published, Deleted int
- TooLarge, NotRegular []string
+ Published, Deleted int
+ TooLarge, NotRegular, Unreadable []string
}
// skipped reports whether rel, or any directory above it, is excluded.
@@ -3249,15 +3499,17 @@ func skipped(skip func(string, bool) bool, rel string, isDir bool) bool {
}
// SyncDir stages every difference between dir and the manifest: changed
-// and new regular files, and a tombstone for each manifest path no longer
-// on disk. It does not commit. skip, when non-nil, excludes a relative
-// path and, for a directory, everything under it; an excluded path is
-// left exactly as the manifest has it.
+// and new regular files, and a tombstone for each manifest path that is
+// no longer there. It does not commit. skip, when non-nil, excludes a
+// relative path and, for a directory, everything under it; an excluded
+// path is left exactly as the manifest has it.
//
-// A root that cannot be read is an error and stages nothing: an
-// unreadable directory is not an empty one.
+// A tombstone needs proof of absence. A file that exists but could not be
+// read keeps its manifest entry and is reported in Unreadable. A root
+// that cannot be opened is an error and stages nothing: an unreadable
+// directory is not an empty one.
func SyncDir(ctx context.Context, p *Publisher, dir string, skip func(rel string, isDir bool) bool) (SyncStats, error) {
- root, err := os.OpenRoot(dir)
+ root, err := openTree(dir)
if err != nil {
return SyncStats{}, fmt.Errorf("hostfiles: sync %s: %w", dir, err)
}
@@ -3265,31 +3517,14 @@ func SyncDir(ctx context.Context, p *Publisher, dir string, skip func(rel string
return syncRoot(ctx, p, root, skip)
}
-func syncRoot(ctx context.Context, p *Publisher, root *os.Root, skip func(rel string, isDir bool) bool) (SyncStats, error) {
+func syncRoot(ctx context.Context, p *Publisher, root *dirHandle, skip func(rel string, isDir bool) bool) (SyncStats, error) {
var st SyncStats
seen := map[string]bool{}
- err := fs.WalkDir(root.FS(), ".", func(rel string, d fs.DirEntry, err error) error {
- if err != nil {
- return err
- }
+ err := syncTree(root, p, skip, func(d *dirHandle, name, rel string) error {
if err := ctx.Err(); err != nil {
return err
}
- if rel == "." {
- return nil
- }
- if skipped(skip, rel, d.IsDir()) {
- if d.IsDir() {
- return fs.SkipDir
- }
- return nil
- }
- if d.IsDir() {
- return nil
- }
- // WalkDir does not follow symlinks: a symlink to a directory
- // arrives here as a non-directory entry, and readStable refuses it.
- body, mode, state, err := readStable(root, rel)
+ body, mode, state, err := readStable(d, name)
if err != nil {
return err
}
@@ -3299,6 +3534,13 @@ func syncRoot(ctx context.Context, p *Publisher, root *os.Root, skip func(rel st
case fileNotRegular:
st.NotRegular = append(st.NotRegular, rel)
return nil
+ case fileUnreadable:
+ seen[rel] = true // it exists: keep what the manifest has
+ st.Unreadable = append(st.Unreadable, rel)
+ return nil
+ case fileRetry:
+ seen[rel] = true // replaced mid-look: the next scan reads it
+ return nil
case fileTooLarge:
seen[rel] = true
st.TooLarge = append(st.TooLarge, rel)
@@ -3328,49 +3570,311 @@ func syncRoot(ctx context.Context, p *Publisher, root *os.Root, skip func(rel st
}
sort.Strings(st.TooLarge)
sort.Strings(st.NotRegular)
+ sort.Strings(st.Unreadable)
return st, nil
}
```
`Publisher.pending()` (in Task 5's `publisher.go`) is the last sequence number handed out; `SyncDir` uses it to count what a `Stage` call really published.
-`internal/hostfiles/open_unix.go`:
+`internal/hostfiles/dir_unix.go`:
```go
//go:build unix
package hostfiles
-import "syscall"
+import (
+ "errors"
+ "io"
+ "os"
+ "sort"
+ "strings"
+
+ "golang.org/x/sys/unix"
+)
+
+// dirHandle is a directory held open by descriptor. Everything beneath it
+// is reached one path component at a time with openat and O_NOFOLLOW, so
+// no symlink is ever followed, inside the tree or out of it, and a
+// directory swapped for something else after it was looked at is refused
+// rather than entered. Every open is non-blocking, so a FIFO swapped in
+// for a file or a directory cannot hang the publisher.
+type dirHandle struct{ f *os.File }
+
+// openTree opens the root of a files directory. The path to the root is
+// the platform's own and may contain symlinks; nothing beneath it may.
+func openTree(path string) (*dirHandle, error) {
+ fd, err := unix.Open(path, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_NONBLOCK, 0)
+ if err != nil {
+ return nil, &os.PathError{Op: "open", Path: path, Err: err}
+ }
+ return &dirHandle{os.NewFile(uintptr(fd), path)}, nil
+}
+
+func (d *dirHandle) Close() error { return d.f.Close() }
+func (d *dirHandle) Name() string { return d.f.Name() }
+
+// openDir opens the directory name inside d. A name that is not a real
+// directory (a symlink, a file, a FIFO) fails with ENOTDIR or ELOOP
+// without blocking; a missing one with ENOENT.
+func (d *dirHandle) openDir(name string) (*dirHandle, error) {
+ fd, err := unix.Openat(int(d.f.Fd()), name, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC|unix.O_NONBLOCK, 0)
+ if err != nil {
+ return nil, &os.PathError{Op: "openat", Path: name, Err: err}
+ }
+ return &dirHandle{os.NewFile(uintptr(fd), name)}, nil
+}
+
+// names lists d's entries, sorted.
+func (d *dirHandle) names() ([]string, error) {
+ if _, err := d.f.Seek(0, io.SeekStart); err != nil {
+ return nil, err
+ }
+ names, err := d.f.Readdirnames(-1)
+ if err != nil {
+ return nil, err
+ }
+ sort.Strings(names)
+ return names, nil
+}
+
+// look inspects name inside d without following a symlink.
+func (d *dirHandle) look(name string) (unix.Stat_t, error) {
+ var st unix.Stat_t
+ err := unix.Fstatat(int(d.f.Fd()), name, &st, unix.AT_SYMLINK_NOFOLLOW)
+ return st, err
+}
+
+func isDir(st unix.Stat_t) bool { return st.Mode&unix.S_IFMT == unix.S_IFDIR }
+func isRegular(st unix.Stat_t) bool { return st.Mode&unix.S_IFMT == unix.S_IFREG }
+
+func sameFile(a, b unix.Stat_t) bool { return a.Dev == b.Dev && a.Ino == b.Ino }
+
+func sameImage(a, b unix.Stat_t) bool {
+ return sameFile(a, b) && a.Size == b.Size && a.Mtim == b.Mtim
+}
+
+// notADirectory reports the errors openDir gives for a name that exists
+// but is not a real directory.
+func notADirectory(err error) bool {
+ return errors.Is(err, unix.ENOTDIR) || errors.Is(err, unix.ELOOP)
+}
-// openNonblock keeps an open from blocking on a FIFO that has no writer.
-// It has no effect on a regular file.
-const openNonblock = syscall.O_NONBLOCK
+// descend opens the directory rel ("" or "a/b") beneath d, one component
+// at a time. gone is true when some component is missing or is not a real
+// directory: nothing of ours lives at a path under it.
+func (d *dirHandle) descend(rel string) (sub *dirHandle, gone bool, err error) {
+ cur, owned := d, false
+ if rel == "" || rel == "." {
+ return d, false, nil
+ }
+ for _, part := range strings.Split(rel, "/") {
+ betweenLookAndOpen()
+ next, err := cur.openDir(part)
+ if owned {
+ cur.Close()
+ }
+ if err != nil {
+ if errors.Is(err, unix.ENOENT) || notADirectory(err) {
+ return nil, true, nil
+ }
+ return nil, false, err
+ }
+ cur, owned = next, true
+ }
+ return cur, false, nil
+}
+
+// readStable reads the file name inside d for publishing.
+//
+// It reads only what it opened without following a link and found to be
+// the same regular file it had just inspected. A file over MaxFileBytes is
+// refused on its size alone, before any read. What it reports:
+//
+// - fileGone: nothing by that name.
+// - fileNotRegular: something by that name that is not a regular file.
+// - fileUnreadable: a regular file that could not be opened or read. It
+// still exists, so the caller must keep what the manifest has for it.
+// - fileRetry: the regular file was replaced by another regular file
+// between the look and the open. Nothing was read; ask again.
+// - fileTorn: read, but its size, mtime or identity changed meanwhile.
+// The bytes are a possibly inconsistent image.
+func readStable(d *dirHandle, name string) ([]byte, uint32, fileState, error) {
+ before, err := d.look(name)
+ if errors.Is(err, unix.ENOENT) {
+ return nil, 0, fileGone, nil
+ }
+ if err != nil {
+ return nil, 0, fileUnreadable, nil
+ }
+ if !isRegular(before) {
+ return nil, 0, fileNotRegular, nil
+ }
+ if before.Size > MaxFileBytes {
+ return nil, 0, fileTooLarge, nil
+ }
+ betweenLookAndOpen()
+ fd, err := unix.Openat(int(d.f.Fd()), name, unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_CLOEXEC|unix.O_NONBLOCK, 0)
+ switch {
+ case errors.Is(err, unix.ENOENT):
+ return nil, 0, fileGone, nil
+ case errors.Is(err, unix.ELOOP):
+ return nil, 0, fileNotRegular, nil // it became a symlink
+ case err != nil:
+ return nil, 0, fileUnreadable, nil
+ }
+ f := os.NewFile(uintptr(fd), name)
+ defer f.Close()
+ var opened unix.Stat_t
+ if err := unix.Fstat(fd, &opened); err != nil {
+ return nil, 0, fileUnreadable, nil
+ }
+ if !isRegular(opened) {
+ return nil, 0, fileNotRegular, nil // swapped for a FIFO, a device, a directory
+ }
+ if !sameFile(before, opened) {
+ return nil, 0, fileRetry, nil
+ }
+ if opened.Size > MaxFileBytes {
+ return nil, 0, fileTooLarge, nil
+ }
+ body, err := readAll(io.LimitReader(f, MaxFileBytes+1))
+ if err != nil {
+ return nil, 0, fileUnreadable, nil
+ }
+ if int64(len(body)) > MaxFileBytes {
+ return nil, 0, fileTooLarge, nil // it grew past the cap while being read
+ }
+ state := fileOK
+ if after, err := d.look(name); err != nil || !sameImage(before, after) {
+ state = fileTorn
+ }
+ return body, uint32(before.Mode & 0o777), state, nil
+}
+```
+
+`internal/hostfiles/walk_unix.go`:
+
+```go
+//go:build unix
+
+package hostfiles
+
+import (
+ "errors"
+
+ "golang.org/x/sys/unix"
+)
+
+// syncTree walks the tree beneath root by descriptor and calls visit for
+// every entry that is not a directory, with the open directory that holds
+// it, its name there, and its path relative to root. It never follows a
+// symlink and never blocks on a FIFO: a directory is entered only through
+// openDir.
+func syncTree(root *dirHandle, p *Publisher, skip func(string, bool) bool, visit func(d *dirHandle, name, rel string) error) error {
+ return walkDir(root, "", skip, visit)
+}
+
+func walkDir(d *dirHandle, prefix string, skip func(string, bool) bool, visit func(d *dirHandle, name, rel string) error) error {
+ names, err := d.names()
+ if err != nil {
+ return err
+ }
+ for _, name := range names {
+ rel := name
+ if prefix != "" {
+ rel = prefix + "/" + name
+ }
+ st, err := d.look(name)
+ if errors.Is(err, unix.ENOENT) {
+ continue
+ }
+ if err != nil {
+ return err
+ }
+ if !isDir(st) {
+ if skipped(skip, rel, false) {
+ continue
+ }
+ if err := visit(d, name, rel); err != nil {
+ return err
+ }
+ continue
+ }
+ if skipped(skip, rel, true) {
+ continue
+ }
+ betweenLookAndOpen()
+ sub, err := d.openDir(name)
+ if err != nil {
+ if errors.Is(err, unix.ENOENT) {
+ continue
+ }
+ if notADirectory(err) {
+ // It was a directory a moment ago and is something else
+ // now. Let readStable say what: it will not follow or
+ // block on it either.
+ if err := visit(d, name, rel); err != nil {
+ return err
+ }
+ continue
+ }
+ return err
+ }
+ err = walkDir(sub, rel, skip, visit)
+ sub.Close()
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
```
-`internal/hostfiles/open_other.go`:
+`internal/hostfiles/dir_other.go`, so the package still builds on Windows, where it restores and reads manifests but does not publish:
```go
//go:build !unix
package hostfiles
-const openNonblock = 0
+import "errors"
+
+// The publisher reads a files directory by descriptor, without following
+// links, which needs openat. The agent runs on Linux; elsewhere this
+// package builds, restores and reads manifests, and does not publish.
+var errNoPublish = errors.New("hostfiles: publishing a directory is not supported on this OS")
+
+type dirHandle struct{}
+
+func openTree(path string) (*dirHandle, error) { return nil, errNoPublish }
+func (d *dirHandle) Close() error { return nil }
+func (d *dirHandle) Name() string { return "" }
+
+func syncTree(d *dirHandle, p *Publisher, skip func(string, bool) bool, visit func(*dirHandle, string, string) error) error {
+ return errNoPublish
+}
+
+func (d *dirHandle) descend(rel string) (*dirHandle, bool, error) { return nil, false, errNoPublish }
+
+func readStable(d *dirHandle, name string) ([]byte, uint32, fileState, error) {
+ return nil, 0, fileUnreadable, errNoPublish
+}
```
Why it is built this way:
-- **`os.Root`** (Go 1.24+) resolves every path beneath one opened directory and refuses any that would leave it, including through a symlink swapped in mid-walk. A path that fails that way is reported as not regular, never read.
-- **`O_NONBLOCK`** stops the open from hanging on a FIFO swapped in for the file. The check after the open (`IsRegular` and `SameFile` against the first look) then refuses it unread.
+- **Everything is reached by descriptor, one component at a time.** `openat` with `O_NOFOLLOW` never follows a symlink, whether it points outside the directory or to another part of it (an excluded directory, say). `O_DIRECTORY` makes an open of something that is not a real directory fail at once, and `O_NONBLOCK` stops any open from hanging on a FIFO. So a directory or file swapped for something else after it was looked at is refused, not entered and not waited on. The scan walks with these handles too: `fs.WalkDir` would re-open each directory by name with a plain blocking open.
+- **A tombstone needs proof of absence.** `readStable` distinguishes five outcomes that are not "here are the bytes", and only two of them stage a delete: the name is gone, or something that is not a regular file stands there. A file that exists but cannot be read (`fileUnreadable`) keeps its manifest entry and is reported. A regular file replaced by another regular file mid-look (`fileRetry`) is read again later.
- **Size is checked before any read**, from the first look and again from the opened file. `TestSyncDirRecordsAFileOverTheCapWithoutReadingIt` counts calls to `readAll` and expects none.
-- **`skipped`** applies an exclusion to a path and to every directory above it, in the walk and in the tombstone pass alike, so excluding a directory never tombstones what the manifest already holds under it.
-
-`SyncDir` returns an error from the walk before it reaches the tombstone loop, so a directory that vanishes or fails partway never tombstones the files it did not reach.
+- **`skipped`** applies an exclusion to a path and to every directory above it, in the walk and in the tombstone pass alike.
+- **`betweenLookAndOpen`** is a test seam that runs in every gap between inspecting a name and opening it. The substitution tests use it to make each race happen exactly once, on demand.
- [ ] **Step 4: Run the tests**
-Run: `go test ./internal/hostfiles/ -count=1`
-Expected: PASS.
+Run: `go test ./internal/hostfiles/ -count=1 && GOOS=windows go build ./internal/hostfiles/ && GOOS=darwin go build ./internal/hostfiles/`
+Expected: PASS, and both cross-builds succeed. Then `go test ./internal/hostfiles/ -run 'Socket|Device' -count=1 -v`: on a machine that allows Unix sockets the socket test must `PASS`, not `SKIP`.
- [ ] **Step 5: Commit**
@@ -3419,6 +3923,7 @@ Spec §5.6, continuous mode: a file is published within one second of its **firs
Three failure rules, each with its own test:
- **Overflow makes coverage uncertain.** A dropped event may have been a directory's creation, which leaves that directory unwatched. So an overflow marks the watcher degraded as well as asking for a scan; the loop then polls and calls `Repair` on every pass until `Repair` succeeds, and scans once more after it does.
+- **A repair clears only what it could have repaired.** Coverage is two counters, not a flag: `failures` rises on every overflow or failed watch, and `repaired` is set to the value `failures` had when a successful walk began. A failure that arrives during the walk may concern a directory the walk has already passed, so it leaves the two unequal and the watcher still degraded.
- **A dead watcher means polling for good.** If the reader stops, its channel closes. The loop sets the channel and the watcher to nil and polls from then on. It must not spin on the closed channel, and `Repair` on a dead watcher must fail.
- **The first full scan is part of start-up.** Tests wait for it (`startRun` returns after the first `OnStats`) before they change anything, and the event tests run with scans and polling an hour apart, so only the mechanism under test can satisfy them.
@@ -3520,6 +4025,8 @@ Run: `go test ./internal/hostfiles/ -run Queue -count=1`. Expected: PASS.
`internal/hostfiles/run_test.go`:
```go
+//go:build unix
+
package hostfiles
import (
@@ -3687,27 +4194,37 @@ func TestRunPollsWhileTheWatcherIsDegradedAndKeepsTryingToRepair(t *testing.T) {
eventually(t, "more than one repair attempt", func() bool { w.mu.Lock(); defer w.mu.Unlock(); return w.repairs > 1 })
}
-// A2-5 residual, the other half: once repair succeeds, polling stops and
-// what changed meanwhile is found by one scan.
+// The other half: once repair succeeds, polling stops. The loop runs
+// with a short polling period, so if it kept polling the scan count would
+// keep climbing; it must scan once more for what changed meanwhile and
+// then go quiet.
func TestRunScansOnceAfterARepairThenStopsPolling(t *testing.T) {
s, dir, w := release.NewDirStore(t.TempDir()), t.TempDir(), newFake()
+ w.setDegraded(true)
+ w.repairErr = errors.New("not yet")
p := open(t, s)
scans := 0
var mu sync.Mutex
- o := eventsOnly
- stop := startRunCounting(t, p, dir, w, o, func() { mu.Lock(); scans++; mu.Unlock() })
+ count := func() int { mu.Lock(); defer mu.Unlock(); return scans }
+ stop := startRunCounting(t, p, dir, w, polled, func() { mu.Lock(); scans++; mu.Unlock() })
defer stop()
+
+ // While repair fails, it polls: the count climbs.
+ n := count()
+ eventually(t, "polling while degraded", func() bool { return count() >= n+3 })
+
write(t, dir, "during.txt", "d")
- w.setDegraded(true) // the fake's Repair succeeds and clears it
+ w.mu.Lock()
+ w.repairErr = nil // the next Repair succeeds and clears degraded
+ w.mu.Unlock()
+ eventually(t, "the repair", func() bool { return !w.Degraded() })
eventually(t, "during.txt found by the post-repair scan", func() bool { return manifestHas(s, "during.txt", sum([]byte("d"))) })
- mu.Lock()
- n := scans
- mu.Unlock()
- time.Sleep(5 * o.Deadline)
- mu.Lock()
- defer mu.Unlock()
- if scans != n {
- t.Fatalf("%d more scans after the repair; polling should have stopped", scans-n)
+
+ time.Sleep(2 * polled.Poll) // let any scan already under way finish
+ n = count()
+ time.Sleep(8 * polled.Poll)
+ if got := count(); got != n {
+ t.Fatalf("%d more scans in eight polling periods after the repair; polling should have stopped", got-n)
}
}
@@ -3782,7 +4299,8 @@ import (
"errors"
"fmt"
"log"
- "os"
+ "path"
+ "strings"
"time"
)
@@ -3827,7 +4345,7 @@ func Run(ctx context.Context, p *Publisher, dir string, o Options) error {
}
func run(ctx context.Context, p *Publisher, dir string, o Options, w watcher) error {
- root, err := os.OpenRoot(dir)
+ root, err := openTree(dir)
if err != nil {
return fmt.Errorf("hostfiles: %s: %w", dir, err)
}
@@ -3913,10 +4431,15 @@ func run(ctx context.Context, p *Publisher, dir string, o Options, w watcher) er
}
}
-// publishOne stages one path's current state. again is true when the file
-// changed while it was read: its image is published flagged, and the
-// caller reads it again at the next deadline.
-func publishOne(ctx context.Context, p *Publisher, root *os.Root, rel string, skip func(string, bool) bool) (again bool, err error) {
+// publishOne stages one path's current state. again is true when the
+// caller should read the path again at the next deadline: the file changed
+// while it was read (its image is published, flagged), or it was replaced
+// mid-look and nothing was read.
+//
+// A tombstone is staged only on proof: the name is gone, or something
+// that is not a regular file now stands there or above it. A file that
+// exists but could not be read is an error, and its manifest entry stays.
+func publishOne(ctx context.Context, p *Publisher, root *dirHandle, rel string, skip func(string, bool) bool) (again bool, err error) {
clean, err := CleanPath(rel)
if err != nil {
return false, nil // a path the watcher should never have produced: skip it
@@ -3924,16 +4447,30 @@ func publishOne(ctx context.Context, p *Publisher, root *os.Root, rel string, sk
if skipped(skip, clean, false) {
return false, nil
}
- body, mode, state, err := readStable(root, clean)
+ parent, name := path.Split(clean)
+ dir, gone, err := root.descend(strings.TrimSuffix(parent, "/"))
+ if err != nil {
+ return false, err
+ }
+ if gone {
+ return false, p.StageDelete(clean)
+ }
+ if dir != root {
+ defer dir.Close()
+ }
+ body, mode, state, err := readStable(dir, name)
if err != nil {
return false, err
}
switch state {
case fileGone, fileNotRegular:
- // Gone, or no longer a regular file (a directory, a link): either
- // way the manifest's file at this path is no more. StageDelete is
- // a no-op for a path the manifest never had.
+ // StageDelete is a no-op for a path the manifest never had, which
+ // is what a directory's own event is.
return false, p.StageDelete(clean)
+ case fileUnreadable:
+ return false, fmt.Errorf("%s exists but could not be read; keeping what is published", clean)
+ case fileRetry:
+ return true, nil
case fileTooLarge:
return false, p.StageSkipped(clean, "too-large")
}
@@ -3978,6 +4515,7 @@ import (
"context"
"os"
"path/filepath"
+ "sync/atomic"
"testing"
"time"
@@ -4071,49 +4609,106 @@ func TestWatcherDoesNotWatchASkippedDirectory(t *testing.T) {
}
}
-// A2-8: a kernel overflow may have swallowed a directory's creation, so
-// coverage is uncertain until Repair has walked the tree again.
-func TestWatcherOverflowMarksCoverageUncertainUntilRepaired(t *testing.T) {
- root := t.TempDir()
- w := realWatcher(t, root, nil).(*inotifyWatcher)
- // The directory appears while events are being dropped: no watch on it.
+// unwatched builds a watcher with one directory that has no watch and no
+// event on its way: the state an overflow leaves when it swallows that
+// directory's creation. The directory exists before the watcher starts
+// and is excluded while the watcher is built, so the kernel never queues
+// an event that could install the watch later behind the test's back.
+func unwatched(t *testing.T) (w *inotifyWatcher, root string) {
+ t.Helper()
+ root = t.TempDir()
if err := os.MkdirAll(filepath.Join(root, "lost"), 0o755); err != nil {
t.Fatal(err)
}
- for len(w.ch) > 0 {
- <-w.ch
+ var building atomic.Bool
+ building.Store(true)
+ w = realWatcher(t, root, func(rel string, isDir bool) bool {
+ return building.Load() && isDir && rel == "lost"
+ }).(*inotifyWatcher)
+ building.Store(false)
+ return w, root
+}
+
+// noEventFor fails if an event for rel arrives within 200ms.
+func noEventFor(t *testing.T, w *inotifyWatcher, rel string) {
+ t.Helper()
+ deadline := time.After(200 * time.Millisecond)
+ for {
+ select {
+ case ev := <-w.Events():
+ if ev.rel == rel {
+ t.Fatalf("an event for %s, which has no watch", rel)
+ }
+ case <-deadline:
+ return
+ }
}
- w.mu.Lock()
- for wd, rel := range w.wds {
- if rel == "lost" {
- delete(w.wds, wd) // as if its IN_CREATE had been dropped
+}
+
+func eventFor(t *testing.T, w *inotifyWatcher, rel string) {
+ t.Helper()
+ deadline := time.After(2 * time.Second)
+ for {
+ select {
+ case ev := <-w.Events():
+ if ev.rel == rel {
+ return
+ }
+ case <-deadline:
+ t.Fatalf("no event for %s", rel)
}
}
- w.mu.Unlock()
+}
+
+// Plan round 1 finding 8: a kernel overflow may have swallowed a
+// directory's creation, so coverage is uncertain until Repair has walked
+// the tree again. Only Repair can restore it here: remove the walk from
+// Repair and the last assertion fails.
+func TestWatcherOverflowMarksCoverageUncertainUntilRepaired(t *testing.T) {
+ w, root := unwatched(t)
+ if w.Degraded() {
+ t.Fatal("degraded before anything went wrong")
+ }
+ write(t, root, "lost/before.txt", "b")
+ noEventFor(t, w, "lost/before.txt") // proves the directory really is unwatched
+
w.handle(0, unix.IN_Q_OVERFLOW, "")
if !w.Degraded() {
t.Fatal("an overflow left coverage marked complete")
}
- if ev := <-w.Events(); !ev.rescan {
- t.Fatalf("event = %+v, want a rescan", ev)
- }
if err := w.Repair(); err != nil {
t.Fatal(err)
}
if w.Degraded() {
t.Fatal("still degraded after a successful repair")
}
- write(t, root, "lost/later.txt", "l")
- deadline := time.After(2 * time.Second)
- for {
- select {
- case ev := <-w.Events():
- if ev.rel == "lost/later.txt" {
- return
- }
- case <-deadline:
- t.Fatal("no event for a write inside the directory whose creation was lost")
- }
+ write(t, root, "lost/after.txt", "a")
+ eventFor(t, w, "lost/after.txt")
+}
+
+// Plan round 2 finding 5: a failure that arrives while Repair is walking
+// may concern a directory the walk has already passed. That Repair must
+// not clear it.
+func TestARepairDoesNotEraseAFailureThatArrivedDuringItsWalk(t *testing.T) {
+ w, _ := unwatched(t)
+ w.handle(0, unix.IN_Q_OVERFLOW, "")
+ orig := afterRepairWalk
+ t.Cleanup(func() { afterRepairWalk = orig })
+ afterRepairWalk = func() {
+ afterRepairWalk = func() {}
+ w.handle(0, unix.IN_Q_OVERFLOW, "") // a second overflow, after the walk
+ }
+ if err := w.Repair(); err != nil {
+ t.Fatal(err)
+ }
+ if !w.Degraded() {
+ t.Fatal("the repair cleared a failure newer than its walk")
+ }
+ if err := w.Repair(); err != nil {
+ t.Fatal(err)
+ }
+ if w.Degraded() {
+ t.Fatal("a repair with nothing new during its walk left the watcher degraded")
}
}
@@ -4169,10 +4764,19 @@ type inotifyWatcher struct {
mu sync.Mutex
wds map[int32]string // watch descriptor -> directory, relative to root ("" is root)
- degraded atomic.Bool // some directory may be unwatched
+ // Coverage is tracked by counting failures, not with a flag. failures
+ // goes up whenever a directory may have been left unwatched; repaired
+ // is the count Repair had seen when it started the walk that
+ // succeeded. A failure that arrives while Repair is walking is
+ // therefore never erased by that Repair: the two differ again.
+ failures atomic.Int64
+ repaired atomic.Int64
dead atomic.Bool // the reader has stopped; no event will ever arrive
}
+// afterRepairWalk runs between Repair's walk and its bookkeeping. A test seam.
+var afterRepairWalk = func() {}
+
func newWatcher(root string, skip func(string, bool) bool) (watcher, error) {
fd, err := unix.InotifyInit1(unix.IN_CLOEXEC | unix.IN_NONBLOCK)
if err != nil {
@@ -4181,15 +4785,17 @@ func newWatcher(root string, skip func(string, bool) bool) (watcher, error) {
w := &inotifyWatcher{root: root, skip: skip, f: os.NewFile(uintptr(fd), "inotify"),
ch: make(chan event, 4096), wds: map[int32]string{}}
if err := w.addTree(""); err != nil {
- w.degraded.Store(true) // partial coverage: Run polls and calls Repair
+ w.failures.Add(1) // partial coverage: Run polls and calls Repair
}
go w.loop()
return w, nil
}
func (w *inotifyWatcher) Events() <-chan event { return w.ch }
-func (w *inotifyWatcher) Degraded() bool { return w.degraded.Load() }
-func (w *inotifyWatcher) Close() error { return w.f.Close() }
+func (w *inotifyWatcher) Degraded() bool {
+ return w.dead.Load() || w.failures.Load() != w.repaired.Load()
+}
+func (w *inotifyWatcher) Close() error { return w.f.Close() }
// Repair re-adds a watch on every directory. InotifyAddWatch on a path
// already watched returns the same descriptor, so this is safe to repeat.
@@ -4198,10 +4804,15 @@ func (w *inotifyWatcher) Repair() error {
if w.dead.Load() {
return errors.New("the inotify reader has stopped")
}
+ seen := w.failures.Load()
if err := w.addTree(""); err != nil {
return err
}
- w.degraded.Store(false)
+ afterRepairWalk()
+ // Only the failures this walk could have covered are cleared. One that
+ // arrived during the walk may concern a directory the walk had already
+ // passed, so it leaves the watcher degraded and Repair runs again.
+ w.repaired.Store(seen)
return nil
}
@@ -4257,7 +4868,7 @@ func (w *inotifyWatcher) send(ev event) {
// Our own buffer is full: the same remedy as a kernel overflow.
// A path was dropped, so only a scan finds it now; and it may
// have been a new directory, so coverage is uncertain too.
- w.degraded.Store(true)
+ w.failures.Add(1)
select {
case <-w.ch:
default:
@@ -4278,7 +4889,6 @@ func (w *inotifyWatcher) loop() {
// Closed by us, or broken. Either way nothing more will be
// read: the closed channel tells Run to poll from here on.
w.dead.Store(true)
- w.degraded.Store(true)
return
}
for off := 0; off+unix.SizeofInotifyEvent <= n; {
@@ -4306,7 +4916,7 @@ func (w *inotifyWatcher) handle(wd int32, mask uint32, name string) {
// Events were dropped, and one of them may have been a new
// directory, which is then unwatched. Coverage is uncertain until
// Repair has walked the tree again.
- w.degraded.Store(true)
+ w.failures.Add(1)
w.send(event{rescan: true})
return
}
@@ -4329,7 +4939,7 @@ func (w *inotifyWatcher) handle(wd int32, mask uint32, name string) {
return
}
if err := w.addTree(rel); err != nil {
- w.degraded.Store(true)
+ w.failures.Add(1)
}
}
// Files may have appeared in the new directory before its watch
@@ -4350,11 +4960,11 @@ func (w *inotifyWatcher) handle(wd int32, mask uint32, name string) {
Run: `gofmt -l internal/hostfiles; go vet ./internal/hostfiles/ && go test ./internal/hostfiles/ -count=1 -race && GOOS=darwin go build ./internal/hostfiles/ && GOOS=windows go build ./internal/hostfiles/`
Expected: no gofmt output, PASS, both cross-builds succeed.
-**The inotify tests skip where the kernel refuses an inotify instance**, and that hides them. Check they ran: `go test ./internal/hostfiles/ -run Watcher -count=1 -v | grep -c SKIP` must print `0`. On the shared development machine it will not: `fs.inotify.max_user_instances` is 128 and the many agent sessions there use them all (this is how the plan's own dry run went: every test here passed except the six real-inotify ones, which skipped). So run them once somewhere with a free instance, with skipping turned off, before this task is called done:
+**The inotify tests skip where the kernel refuses an inotify instance**, and that hides them. Check they ran: `go test ./internal/hostfiles/ -run Watcher -count=1 -v | grep -c SKIP` must print `0`. On the shared development machine it will not: `fs.inotify.max_user_instances` is 128 and the many agent sessions there use them all (this is how the plan's own dry run went: every test here passed except the real-inotify ones, which skipped). So run them once somewhere with a free instance, with skipping turned off, before this task is called done:
CARLOS_REQUIRE_INOTIFY=1 go test ./internal/hostfiles/ -run Watcher -count=1 -race -v
-Expected: six tests `PASS`, none `SKIP`. The dev box is the place; ask Paul for the session. Until that run is green, say in the task's report that `watch_linux.go` is unverified against a real kernel. Do not raise the machine's inotify limit yourself: that is Paul's call.
+Expected: seven tests `PASS`, none `SKIP`. The dev box is the place; ask Paul for the session. Until that run is green, say in the task's report that `watch_linux.go` is unverified against a real kernel. Do not raise the machine's inotify limit yourself: that is Paul's call.
If `TestWatcherPublishesAFileStillHeldOpen` is flaky under `-race`, do not raise `eventually`'s two-second bound past five seconds: look for a lost event first.
@@ -5868,3 +6478,23 @@ Findings are in `CODEX-PLAN-REGION-HA-1A-R1.md`. All nineteen are accepted. Afte
| 19 | P3 Task 5: storage protection attributed to Task 11 | fixed: it is plan 1b's, and must precede switching the publisher on |
**What the dry run could not check**, and the plan says so at each place: the six tests that need a real inotify instance (the development machine is at its per-user limit of 128), and the IMDS signature against a real box.
+
+## Astra plan round 2: dispositions
+
+Findings are in `CODEX-PLAN-REGION-HA-1A-R2.md`. Round 2 marked twelve of round 1's nineteen resolved and raised seven. All seven, and the residuals, are accepted. The fixes were dry-run again.
+
+| # | Finding | Disposition |
+|---|---|---|
+| 1 | P1 Task 6: the walk can block on a directory swapped for a FIFO | fixed: the walk is by descriptor (`walk_unix.go`), entering a directory only through `openat` with `O_DIRECTORY|O_NOFOLLOW|O_NONBLOCK`; `TestSyncDirDoesNotBlockOnADirectorySwappedForAFIFO` |
+| 2 | P1 Tasks 6, 7: read failures became tombstones | fixed: `fileUnreadable` keeps the manifest entry and is reported; `fileRetry` for a regular file replaced mid-look; a tombstone only on proof. Two tests |
+| 3 | P1 Task 5: restore not crash-durable | fixed: every directory in which a name changed is synced before `Restore` returns, deepest first, and a failed sync fails the restore; tested through a seam |
+| 4 | P2 Tasks 6, 7: in-root ancestor symlinks bypass exclusions | fixed by the same descriptor traversal: no symlink is followed anywhere. `TestPublishOneNeverReadsThroughASwappedAncestor` covers a link to outside and a link to an excluded directory, through the queued-event path |
+| 5 | P2 Task 7: a repair can erase a newer failure | fixed: `failures` and `repaired` counters; tested with a failure injected after the walk |
+| 6 | P2 Task 4: tie reporting depended on self and on the winner | fixed: `tiedStamps` looks at every file and every pair; tests for `9, 7, 7` and for a tie with this box itself |
+| 7 | P2 Task 7: two repair tests could pass with their mechanism off | fixed: the scan-count test polls at 40 ms, proves polling while degraded, then requires no scan across eight periods after the repair; the overflow test builds an unwatched directory with no pending kernel event, proves it is unwatched, and only then repairs |
+| R1-4 | residual: in-root links | finding 4 above |
+| R1-8 | residual: overflow repair | findings 5 and 7 above |
+| R1-15 | residual: socket and device cases | added; they skip where the sandbox forbids sockets or the user is not root. The socket test was run outside the sandbox and passes |
+| R1-18 | residual: diagnostics | finding 6 above |
+
+**Still not run:** the seven tests that need a real inotify instance (the machine is at its per-user limit), and the IMDS signature check.
diff --git a/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-BRIEF-3.md b/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-BRIEF-3.md
new file mode 100644
index 000000000..cc5466728
--- /dev/null
+++ b/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-BRIEF-3.md
@@ -0,0 +1,41 @@
+# Review brief: region HA plan 1a (foundations), round 3
+
+Read `CODEX-PLAN-REGION-HA-1A-BRIEF-1.md` first: its "Fixed: do not
+re-argue" and "What to check" sections still apply, to the plan at the
+commit named in the prompt.
+
+## Round 3
+
+This revision answers your round 2 (`CODEX-PLAN-REGION-HA-1A-R2.md`); the
+plan's last section is its dispositions table. The fixes were dry-run
+again: the host-files package and the home-map change were written into
+the tree, built for linux, darwin and windows, and tested with the race
+detector, then removed. The `internal/hostfiles` code blocks are those
+files verbatim.
+
+First give a disposition for your seven round 2 findings and the four
+round 1 residuals: resolved, partly resolved (say what is left), or not
+resolved. Then report only new findings, including any the fixes
+introduce. Look in particular at:
+
+- `dir_unix.go` and `walk_unix.go`: the descriptor traversal. Any
+ remaining way to follow a link, to block, to leak a descriptor, or to
+ mis-handle `Readdirnames` on a reused handle. `descend`'s ownership of
+ the handles it opens. Whether `readStable`'s five non-OK outcomes are
+ each handled correctly by both callers (`syncRoot` and `publishOne`).
+- `Restore`'s directory syncs: is any directory whose entries changed
+ left unsynced, including a parent whose child directory was created or
+ whose obstructing file or symlink was removed?
+- The watcher's `failures`/`repaired` counters, including a failure that
+ arrives between `Repair` reading `failures` and starting its walk.
+- `tiedStamps` and whether `foldHomes`'s routing result is unchanged from
+ the tree's today for every unstamped input.
+- Whether each new or rewritten test fails when its mechanism is
+ disabled.
+
+Do not re-raise anything you mark resolved. The author still cannot run
+the seven real-inotify tests on the development machine and says so.
+
+## Output
+
+As round 1. End with a verdict: ready to build, or not ready.