moves: S1 sends the declared readiness path verbatim; archive and budget errors reach reports only as recorded sentences (Task 14 fix round 2)
- askInstance keeps the probe URL's RawPath: /ready%2Fcheck is asked as declared. - archiveFailure picks M-C14, M-C15, MV-N79, MV-N81 or MV-N151 by errors.Is, the path from the new archive.PathError, and logs the raw error; nothing of an error's text reaches S1. - BudgetCheck builds MV-N68 from moves.NoFreshRateError's Which. - Copy MV-N151, MV-N152; movesCopyIDs extended (that hunk only).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
10 files changed,
+234
−21
approved_moves_copy_test.go+1 −0docs/superpowers/plans/2026-10-06-moves-phase1-copy.md+2 −0internal/host/movesphase1.go+35 −12internal/host/movesphase1_test.go+71 −0internal/host/movesprobe.go+5 −2internal/host/movesprobe_test.go+20 −0internal/moves/archive/archive.go+27 −4internal/moves/archive/archive_unix_test.go+41 −0internal/moves/readiness.go+17 −3internal/moves/readiness_test.go+15 −0
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index ebcf37d53..15d3c3bf1 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -65,6 +65,7 @@ var movesCopyIDs = []string{ |
| "MV-N133", "MV-N134", "MV-N135", "MV-N136", "MV-N137", "MV-N138", |
| "MV-N139", "MV-N140", "MV-N141", "MV-N142", "MV-N143", "MV-N144", |
| "MV-N145", "MV-N146", "MV-N147", "MV-N148", "MV-N149", "MV-N150", |
| + "MV-N151", "MV-N152", |
| } |
| // copyRow is one row of the record. |
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md| index c94545d59..c0de569b4 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| @@ -190,6 +190,8 @@ Columns: id, file, text, status. |
| | MV-N148 | internal/host/movesphase1.go | the app's readiness path could not be read | pending | |
| | MV-N149 | internal/moves/readiness.go | the target has not checked a release yet | pending | |
| | MV-N150 | internal/moves/readiness.go | the target checked release <release> and the source now runs <release> | pending | |
| +| MV-N151 | internal/host/movesphase1.go | the files beside the database could not be archived | pending | |
| +| MV-N152 | internal/moves/readiness.go | the cut budget could not be priced | pending | |
| ## Plan 2 |
diff --git a/internal/host/movesphase1.go b/internal/host/movesphase1.go| index e43a22acc..591ccda12 100644 |
| --- a/internal/host/movesphase1.go |
| +++ b/internal/host/movesphase1.go |
| @@ -76,6 +76,19 @@ func s1OverCap(host string) string { |
| return host + " keeps more than 256 MiB of files beside its database. Keep large files in object storage." |
| } |
| +// s1TooManyEntries and s1LongPath are MV-N79 and MV-N81, the archive's own |
| +// rows, written here from data (the sentinel, the path the error carries) |
| +// rather than taken from an error's text. s1ArchiveFailed is every archive |
| +// error this round does not recognise; its text goes to the log only. |
| +const ( |
| + s1TooManyEntries = "archive: the files directory holds more than 131072 entries" |
| + s1ArchiveFailed = "the files beside the database could not be archived" |
| +) |
| + |
| +func s1LongPath(path string) string { |
| + return "archive: refused path: " + path + " is longer than 4096 bytes" |
| +} |
| + |
| // s1NoAnswer is M-C16. |
| func s1NoAnswer(host string) string { |
| return host + " does not answer its readiness path. Declare moves.ready_path for this app." |
| @@ -536,24 +549,34 @@ func movesScratch(dataDir, pattern string) (string, error) { |
| } |
| // archiveFailure maps an archive error to an S1 reason (a standing refusal |
| -// the operator must act on), a retry (a file changed under the walk), or an |
| -// error that ends the round. |
| +// the operator must act on), a retry (a file changed under the walk), or |
| +// nothing. The reason is always a recorded sentence chosen by errors.Is, |
| +// with a path only from the *archive.PathError the error carries; the |
| +// error's own text, which can name anything the walk met, is logged and |
| +// never published (ruling T15-sinks). An error this does not recognise |
| +// gets s1ArchiveFailed. |
| func archiveFailure(host string, err error) (why string, retry bool, ferr error) { |
| - switch { |
| - case err == nil: |
| + if err == nil { |
| return "", false, nil |
| - case errors.Is(err, archive.ErrChanged): |
| + } |
| + if errors.Is(err, archive.ErrChanged) { |
| log.Printf("moves: %s: %v; the archive is rebuilt next round", host, err) |
| return "", true, nil |
| - case errors.Is(err, archive.ErrSpecialFile): |
| - path := strings.TrimPrefix(err.Error(), archive.ErrSpecialFile.Error()+": ") |
| - return s1SpecialFile(host, strings.TrimSuffix(path, " is a hard link")), false, nil |
| + } |
| + log.Printf("moves: %s: archive the files: %v", host, err) |
| + var pe *archive.PathError |
| + hasPath := errors.As(err, &pe) && pe.Path != "" |
| + switch { |
| + case errors.Is(err, archive.ErrSpecialFile) && hasPath: |
| + return s1SpecialFile(host, pe.Path), false, nil // M-C14, hard links too |
| case errors.Is(err, archive.ErrOverCap): |
| - return s1OverCap(host), false, nil |
| - case errors.Is(err, archive.ErrTooManyEntries), errors.Is(err, archive.ErrBadPath): |
| - return err.Error(), false, nil // MV-N79, MV-N81 |
| + return s1OverCap(host), false, nil // M-C15 |
| + case errors.Is(err, archive.ErrTooManyEntries): |
| + return s1TooManyEntries, false, nil // MV-N79 |
| + case errors.Is(err, archive.ErrBadPath) && hasPath: |
| + return s1LongPath(pe.Path), false, nil // MV-N81 |
| } |
| - return "", false, fmt.Errorf("archive the files: %w", err) |
| + return s1ArchiveFailed, false, nil |
| } |
| // digestHex is "sha256:<hex>" without its algorithm, for an object name. |
diff --git a/internal/host/movesphase1_test.go b/internal/host/movesphase1_test.go| index 31a873d35..dd623d13b 100644 |
| --- a/internal/host/movesphase1_test.go |
| +++ b/internal/host/movesphase1_test.go |
| @@ -711,3 +711,74 @@ func TestS1ReadsTheDeclaredReadinessPath(t *testing.T) { |
| t.Fatalf("the instance was probed at %v with no readable manifest", h.probePaths[asked:]) |
| } |
| } |
| + |
| +// Archive errors reach S1 only as recorded sentences, chosen by errors.Is |
| +// and the path the error carries as data; the raw error text, which may |
| +// name anything, goes to the log alone (ruling T15-sinks). |
| +func TestArchiveErrorsReachS1AsRecordedSentences(t *testing.T) { |
| + secret := "EIO reading /data/other-tenant/secret" |
| + for name, tc := range map[string]struct { |
| + err error |
| + want string |
| + }{ |
| + "special file": {&archive.PathError{Err: archive.ErrSpecialFile, Path: "link"}, s1SpecialFile(p1Host, "link")}, |
| + "over cap": {fmt.Errorf("walk: %w", archive.ErrOverCap), s1OverCap(p1Host)}, |
| + "too many": {archive.ErrTooManyEntries, s1TooManyEntries}, |
| + "long path": {&archive.PathError{Err: archive.ErrBadPath, Path: "deep"}, s1LongPath("deep")}, |
| + "special, no path": {fmt.Errorf("%w: %s", archive.ErrSpecialFile, secret), s1ArchiveFailed}, |
| + "bad path, no path": {fmt.Errorf("%w: %s", archive.ErrBadPath, secret), s1ArchiveFailed}, |
| + "unrecognised": {errors.New(secret), s1ArchiveFailed}, |
| + } { |
| + t.Run(name, func(t *testing.T) { |
| + h := newPhase1Harness(t) |
| + h.start(t, "manual") |
| + prev := archivePlainDigest |
| + t.Cleanup(func() { archivePlainDigest = prev }) |
| + archivePlainDigest = func(string, string) (string, int64, int, error) { return "", 0, 0, tc.err } |
| + logs := captureLog(t) |
| + h.pass(t) |
| + sr := h.sourceReport(t) |
| + if sr == nil || sr.S1.OK || sr.S1.Reason != tc.want { |
| + t.Fatalf("S1 = %+v, want %q", sr, tc.want) |
| + } |
| + if strings.Contains(sr.S1.Reason, "EIO") || strings.Contains(sr.S1.Reason, "other-tenant") { |
| + t.Fatalf("the raw error reached S1: %q", sr.S1.Reason) |
| + } |
| + if !strings.Contains(logs(), tc.err.Error()) { |
| + t.Fatalf("the raw error was not logged:\n%s", logs()) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// MV-N79 and MV-N81 are the archive's own texts, written from data. |
| +func TestArchiveSentencesMatchTheArchiveRows(t *testing.T) { |
| + if s1TooManyEntries != archive.ErrTooManyEntries.Error() { |
| + t.Fatalf("%q vs %q", s1TooManyEntries, archive.ErrTooManyEntries.Error()) |
| + } |
| + // A relative path over MaxPathLen, made one directory at a time |
| + // through os.Root so no single call meets the OS path limit. |
| + files := t.TempDir() |
| + long := strings.Repeat("d", 200) |
| + cur, err := os.OpenRoot(files) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for depth := 0; depth*(len(long)+1) <= archive.MaxPathLen; depth++ { |
| + if err := cur.Mkdir(long, 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + next, err := cur.OpenRoot(long) |
| + cur.Close() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cur = next |
| + } |
| + cur.Close() |
| + _, _, _, err = archive.PlainDigest(files, "") |
| + var pe *archive.PathError |
| + if !errors.As(err, &pe) || !errors.Is(err, archive.ErrBadPath) || err.Error() != s1LongPath(pe.Path) { |
| + t.Fatalf("err %v; s1LongPath %q", err, s1LongPath(pe.Path)) |
| + } |
| +} |
diff --git a/internal/host/movesprobe.go b/internal/host/movesprobe.go| index 49f6a3b49..b7fd389cc 100644 |
| --- a/internal/host/movesprobe.go |
| +++ b/internal/host/movesprobe.go |
| @@ -8,7 +8,6 @@ import ( |
| "log" |
| "net" |
| "net/http" |
| - "net/url" |
| "strings" |
| "time" |
| @@ -92,7 +91,11 @@ func askInstance(ctx context.Context, socket string, r *http.Request) (int, erro |
| }, |
| CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, |
| } |
| - out, err := http.NewRequestWithContext(ctx, http.MethodGet, (&url.URL{Scheme: "http", Host: r.Host, Path: r.URL.Path, RawQuery: r.URL.RawQuery}).String(), nil) |
| + // The declared target as declared: the URL keeps RawPath, so an |
| + // escaped "/ready%2Fcheck" is never sent as "/ready/check". |
| + u := *r.URL |
| + u.Scheme, u.Host = "http", r.Host |
| + out, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) |
| if err != nil { |
| return 0, err |
| } |
diff --git a/internal/host/movesprobe_test.go b/internal/host/movesprobe_test.go| index 7dd7b8012..0f7f88d97 100644 |
| --- a/internal/host/movesprobe_test.go |
| +++ b/internal/host/movesprobe_test.go |
| @@ -124,3 +124,23 @@ func TestS1ProbeNeedsTheInstanceItself(t *testing.T) { |
| t.Fatalf("an always-on instance: %+v", c) |
| } |
| } |
| + |
| +// The declared readiness path reaches the instance as declared: an escaped |
| +// slash stays escaped (Task 14 re-review), so an app that tells |
| +// /ready%2Fcheck from /ready/check is asked the one it declared. |
| +func TestS1ProbeSendsTheDeclaredPathVerbatim(t *testing.T) { |
| + var got []string |
| + rt := instanceForTest(t, func(w http.ResponseWriter, r *http.Request) { |
| + got = append(got, r.RequestURI) |
| + w.WriteHeader(http.StatusOK) |
| + }) |
| + for _, path := range []string{"/ready%2Fcheck", "/ready?probe=a%2Fb", "/plain"} { |
| + got = nil |
| + if c := probeInstance(context.Background(), wakeThenProxy, rt, path); !c.OK { |
| + t.Fatalf("%s: %+v", path, c) |
| + } |
| + if len(got) != 1 || got[0] != path { |
| + t.Fatalf("declared %q, the instance received %q", path, got) |
| + } |
| + } |
| +} |
diff --git a/internal/moves/archive/archive.go b/internal/moves/archive/archive.go| index f31ae6aad..08fa24ece 100644 |
| --- a/internal/moves/archive/archive.go |
| +++ b/internal/moves/archive/archive.go |
| @@ -44,6 +44,29 @@ var ( |
| ErrNotEmpty = errors.New("archive: the destination files directory is not empty") |
| ) |
| +// PathError is a Build refusal that names a file: Err is the sentinel |
| +// (errors.Is sees it), Path the file as Build names it (relative to the |
| +// files directory, or the env file's base name). It lets a caller say |
| +// which file in its own words without reading the error's text; Error() is |
| +// the text the sentinel's rows record (MV-N77, MV-N81, MV-N86). |
| +type PathError struct { |
| + Err error |
| + Path string |
| + msg string |
| +} |
| + |
| +func (e *PathError) Error() string { |
| + if e.msg != "" { |
| + return e.msg |
| + } |
| + return e.Err.Error() + ": " + e.Path |
| +} |
| +func (e *PathError) Unwrap() error { return e.Err } |
| + |
| +func pathError(sentinel error, path, suffix string) error { |
| + return &PathError{Err: sentinel, Path: path, msg: sentinel.Error() + ": " + path + suffix} |
| +} |
| + |
| // The limits as enforced. Variables only so tests can lower them; nothing |
| // else writes them. |
| var ( |
| @@ -119,10 +142,10 @@ func walk(root *os.Root, envPath string) ([]item, error) { |
| return err |
| } |
| if !info.Mode().IsRegular() && !info.IsDir() { |
| - return fmt.Errorf("%w: %s", ErrSpecialFile, p) |
| + return pathError(ErrSpecialFile, p, "") |
| } |
| if len(p) > MaxPathLen { |
| - return fmt.Errorf("%w: %s is longer than %d bytes", ErrBadPath, p, MaxPathLen) |
| + return pathError(ErrBadPath, p, fmt.Sprintf(" is longer than %d bytes", MaxPathLen)) |
| } |
| return add(item{name: FilesPrefix + p, rel: p, root: root, info: info}) |
| }) |
| @@ -137,7 +160,7 @@ func walk(root *os.Root, envPath string) ([]item, error) { |
| case err != nil: |
| return nil, err |
| case !info.Mode().IsRegular(): |
| - return nil, fmt.Errorf("%w: %s", ErrSpecialFile, filepath.Base(envPath)) |
| + return nil, pathError(ErrSpecialFile, filepath.Base(envPath), "") |
| default: |
| if err := add(item{name: EnvEntry, path: envPath, info: info}); err != nil { |
| return nil, err |
| @@ -184,7 +207,7 @@ func openItemChecked(it item) (io.ReadCloser, error) { |
| // carry the outside inode to the target. Counted on the opened file. |
| if linkCount(got) > 1 { |
| f.Close() |
| - return nil, fmt.Errorf("%w: %s is a hard link", ErrSpecialFile, displayName(it)) |
| + return nil, pathError(ErrSpecialFile, displayName(it), " is a hard link") |
| } |
| return f, nil |
| } |
diff --git a/internal/moves/archive/archive_unix_test.go b/internal/moves/archive/archive_unix_test.go| index 4b2d829e9..caab62dde 100644 |
| --- a/internal/moves/archive/archive_unix_test.go |
| +++ b/internal/moves/archive/archive_unix_test.go |
| @@ -94,3 +94,44 @@ func TestBuildRefusesHardLinks(t *testing.T) { |
| t.Fatalf("two links inside: %v", err) |
| } |
| } |
| + |
| +// A Build refusal that names a file carries the path as data |
| +// (*archive.PathError), so a caller can say which file without reading the |
| +// error's text; errors.Is and the text are unchanged. |
| +func TestBuildRefusalsCarryTheirPath(t *testing.T) { |
| + for name, tc := range map[string]struct { |
| + setup func(t *testing.T, files string) |
| + sentinel error |
| + path string |
| + }{ |
| + "symlink": {func(t *testing.T, files string) { |
| + if err := os.Symlink("/etc/passwd", filepath.Join(files, "link")); err != nil { |
| + t.Fatal(err) |
| + } |
| + }, ErrSpecialFile, "link"}, |
| + "hard link": {func(t *testing.T, files string) { |
| + if err := os.WriteFile(filepath.Join(files, "a"), []byte("x"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Link(filepath.Join(files, "a"), filepath.Join(files, "b")); err != nil { |
| + t.Fatal(err) |
| + } |
| + }, ErrSpecialFile, "a"}, |
| + } { |
| + t.Run(name, func(t *testing.T) { |
| + files := filepath.Join(t.TempDir(), "files") |
| + if err := os.MkdirAll(files, 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + tc.setup(t, files) |
| + _, _, _, err := PlainDigest(files, "") |
| + var pe *PathError |
| + if !errors.Is(err, tc.sentinel) || !errors.As(err, &pe) || pe.Path != tc.path { |
| + t.Fatalf("err %v, path error %+v", err, pe) |
| + } |
| + if !strings.Contains(err.Error(), tc.sentinel.Error()+": ") { |
| + t.Fatalf("the text changed: %q", err) |
| + } |
| + }) |
| + } |
| +} |
diff --git a/internal/moves/readiness.go b/internal/moves/readiness.go| index b804597bd..9d5b8e8aa 100644 |
| --- a/internal/moves/readiness.go |
| +++ b/internal/moves/readiness.go |
| @@ -327,15 +327,25 @@ func (m Measurements) Rates(now time.Time, barBytes int64) (Rates, error) { |
| {"the disk", m.Disk}, |
| } { |
| if !fresh(c.m.MiBPerS, c.m.At, now) { |
| - return Rates{}, fmt.Errorf("no fresh rate measurement for %s", c.which) |
| + return Rates{}, &NoFreshRateError{Which: c.which} |
| } |
| } |
| if !fresh(m.Fsync.PerS, m.Fsync.At, now) { |
| - return Rates{}, fmt.Errorf("no fresh rate measurement for %s", "the disk's per-file sync") |
| + return Rates{}, &NoFreshRateError{Which: "the disk's per-file sync"} |
| } |
| return Rates{Cross: m.Cross.MiBPerS, S3: m.S3.MiBPerS, Disk: m.Disk.MiBPerS, MirrorBar: BarMiB(barBytes), FsyncRate: m.Fsync.PerS}, nil |
| } |
| +// NoFreshRateError is Measurements.Rates refusing a rate: Which names it. |
| +type NoFreshRateError struct{ Which string } |
| + |
| +func (e *NoFreshRateError) Error() string { return NoFreshRate(e.Which) } |
| + |
| +// NoFreshRate is T11's sentence for a missing or stale rate (MV-N68). |
| +func NoFreshRate(which string) string { |
| + return fmt.Sprintf("no fresh rate measurement for %s", which) |
| +} |
| + |
| // fresh is a usable rate measured within MeasurementFreshness of now and no |
| // more than MeasurementSkew ahead of it. |
| func fresh(rate float64, at, now time.Time) bool { |
| @@ -370,8 +380,12 @@ func BudgetCheck(dbMiB float64, entries int, m Measurements, barBytes int64, win |
| return fail("the cut window is not a usable number of seconds") |
| } |
| rates, err := m.Rates(now, barBytes) |
| + var nf *NoFreshRateError |
| + if errors.As(err, &nf) { |
| + return fail(NoFreshRate(nf.Which)) |
| + } |
| if err != nil { |
| - return fail(err.Error()) |
| + return fail("the cut budget could not be priced") |
| } |
| e := Estimate(dbMiB, entries, rates) |
| n := e.AttemptsAdmitted(windowSeconds) |
diff --git a/internal/moves/readiness_test.go b/internal/moves/readiness_test.go| index 1af96f089..53669edfb 100644 |
| --- a/internal/moves/readiness_test.go |
| +++ b/internal/moves/readiness_test.go |
| @@ -825,3 +825,18 @@ func TestReadyNeedsTheReleaseTheTargetChecked(t *testing.T) { |
| t.Fatal("two empty releases promoted") |
| } |
| } |
| + |
| +// Measurements.Rates names the stale rate as data (*NoFreshRateError), and |
| +// BudgetCheck builds its reason from that, never from an error's text. |
| +func TestNoFreshRateIsTyped(t *testing.T) { |
| + m := goodMeasurements() |
| + m.S3 = MeasuredRate{} |
| + _, err := m.Rates(t11Now, 16<<20) |
| + var nf *NoFreshRateError |
| + if !errors.As(err, &nf) || nf.Which != "regional S3" { |
| + t.Fatalf("%v", err) |
| + } |
| + if r := BudgetCheck(100, 0, m, 16<<20, CutWindowDefault, t11Now); r.Check.Reason != NoFreshRate("regional S3") { |
| + t.Fatalf("%q", r.Check.Reason) |
| + } |
| +} |