carlos / platform Public

offline-move: carlos move offline fix round 2

The diagnostic allow-list covers only the two err.Error() Reason writes in
retire (exact count 2); the three fixed-text reasons are scanned and
recorded as pending rows MV-O59 to MV-O62, with MV-O63/64 for the two
error-ending forms. A failed report seen in open, finishing or reverting
now shows in status (exit 4) and stops open, finish and abort waits:
MV-O6 for the target, MV-O9 for the sealer, new MV-O65 for a source box.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

claude 5 days ago 29cce53d6ee2af18fe436c44fff47f997e7ef1b3 parent b865d02

5 files changed, +163 −35

approved_moves_copy_scan_test.go +16 −1
approved_moves_copy_test.go +23 −14
docs/superpowers/plans/2026-10-06-moves-phase1-copy.md +12 −0
move.go +62 −20
move_test.go +50 −0

diff --git a/approved_moves_copy_scan_test.go b/approved_moves_copy_scan_test.go

index 27019c88b..2de558d4d 100644

--- a/approved_moves_copy_scan_test.go

+++ b/approved_moves_copy_scan_test.go

@@ -378,6 +378,21 @@ type movesUniverse struct {

diagSkipped map[string]int

}

+// callsError reports whether e calls an Error method: the write carries an

+// error's text. Only such writes can be listed in movesDiagnosticWrites.

+func callsError(e ast.Expr) bool {

+ found := false

+ ast.Inspect(e, func(n ast.Node) bool {

+ if c, ok := n.(*ast.CallExpr); ok {

+ if sel, ok := c.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Error" {

+ found = true

+ }

+ return !found

+ })

+ return found

func (u *movesUniverse) at(p string) string {

if u.root == "." {

return p

@@ -472,7 +487,7 @@ func newUniverse(t *testing.T, root string, files []string) *movesUniverse {

if kv, ok := el.(*ast.KeyValueExpr); ok {

if k, ok := kv.Key.(*ast.Ident); ok {

if fd != nil {

- if key := fd.Name.Name + "|" + ctype + "." + k.Name; movesDiagnosticWrites[f][key] != "" {

+ if key := fd.Name.Name + "|" + ctype + "." + k.Name; movesDiagnosticWrites[f][key].Why != "" && callsError(kv.Value) {

u.diagSkipped[f+"|"+key]++

continue

}

diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go

index 1622610d5..490e1bb38 100644

--- a/approved_moves_copy_test.go

+++ b/approved_moves_copy_test.go

@@ -96,7 +96,7 @@ var movesCopyIDs = []string{

"MV-O20", "MV-O21", "MV-O22", "MV-O23", "MV-O24", "MV-O25", "MV-O26", "MV-O27", "MV-O28", "MV-O29", "MV-O30", "MV-O31",

"MV-O32", "MV-O33",

"MV-O34", "MV-O35", "MV-O36", "MV-O37", "MV-O38", "MV-O39", "MV-O40", "MV-O41", "MV-O42", "MV-O43",

- "MV-O50", "MV-O51", "MV-O52", "MV-O53", "MV-O54", "MV-O55", "MV-O56", "MV-O57", "MV-O58",

+ "MV-O50", "MV-O51", "MV-O52", "MV-O53", "MV-O54", "MV-O55", "MV-O56", "MV-O57", "MV-O58", "MV-O59", "MV-O60", "MV-O61", "MV-O62", "MV-O63", "MV-O64", "MV-O65",

}

// copyRow is one row of the record. The File cell is the file the sentence is

@@ -1528,14 +1528,23 @@ var movesSinkAllow = map[string]map[string]string{

// movesDiagnosticWrites are field writes the scan leaves out of a field's

// writes because what they write is an OS diagnostic for the operator's

// eye, not copy. Paul's rule (2026-10-16): internal error fragments and

-// diagnostics stay out of copy review. The key is "<function>|<struct>.<field>"

-// and the value says why. An entry leaves out only the composite-literal

-// writes of that field in that function of that file, so the field's other

-// writers (the Phase 1 Reason reads in move.go among them) are judged as before.

-// TestMovesDiagnosticWritesAreReal holds each entry to a write it leaves out.

-var movesDiagnosticWrites = map[string]map[string]string{

+// diagnostics stay out of copy review. The key is "<function>|<struct>.<field>".

+// An entry leaves out only the composite-literal writes of that field in that

+// function of that file whose value calls Error(), and says how many there are

+// (Count), so the field's other writes in the same function stay scanned, and

+// so do its other writers (the Phase 1 Reason reads in move.go among them).

+// TestMovesDiagnosticWritesAreReal holds each entry to exactly that many.

+type movesDiagnosticWrite struct {

+ Why string

+ Count int

+var movesDiagnosticWrites = map[string]map[string]movesDiagnosticWrite{

"internal/host/movesoffline.go": {

- "retire|RetiredPath.Reason": "the parked report's reason for a path left in place is the OS error of the rename (leftCannot or leftNotYet plus err.Error()), a box diagnostic that `carlos move offline status` shows as MV-O58's <reason>; it is not copy",

+ "retire|RetiredPath.Reason": {

+ Why: "the parked report's reason for a path left in place ends in the OS error of the rename (leftCannot or leftNotYet plus err.Error()), a box diagnostic that `carlos move offline status` shows as MV-O58's <reason>; it is not copy",

+ Count: 2,

+ },

}

@@ -2487,17 +2496,17 @@ func TestMovesServerTextRefusesForgedValues(t *testing.T) {

}

-// Every movesDiagnosticWrites entry leaves out a write that exists, and the

-// reason it gives is a sentence.

+// Every movesDiagnosticWrites entry leaves out exactly the writes it counts,

+// and the reason it gives is a sentence.

func TestMovesDiagnosticWritesAreReal(t *testing.T) {

u := newUniverse(t, ".", movesScanFiles)

for f, keys := range movesDiagnosticWrites {

- for key, why := range keys {

- if len(why) < 40 {

+ for key, w := range keys {

+ if len(w.Why) < 40 {

t.Errorf("%s %s: the reason is too short to be one", f, key)

}

- if u.diagSkipped[f+"|"+key] == 0 {

- t.Errorf("%s %s leaves out no write: drop the entry", f, key)

+ if got := u.diagSkipped[f+"|"+key]; got != w.Count {

+ t.Errorf("%s %s leaves out %d writes, want exactly %d", f, key, got, w.Count)

}

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 e84f331bf..2cdb820ff 100644

--- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md

+++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md

@@ -554,3 +554,15 @@ Seven more lines of `carlos move offline` (`move.go`, Task 7), for places MV-O1

+The source box's reasons for a path it left in place (`internal/host/movesoffline.go`, `retire`), which `carlos move offline status` shows as MV-O58's `<reason>`. They are human text the box writes into its parked report. The two that end in the box's own OS error (`<error>`, a diagnostic, not copy) are recorded for their fixed beginning.

+|---|---|---|---|

diff --git a/move.go b/move.go

index 161000c9e..e895c66c0 100644

--- a/move.go

+++ b/move.go

@@ -1110,11 +1110,35 @@ func (r offlineReportView) reason() string {

return r.Phase

}

-// failure says whether a box has failed its step, which role it was (the

-// target landing, else the sealer sealing) and why: the items the role's

-// report failed, else what its failed report says, else the word failed.

-// Both roles at once go by the state the move is in.

-func (v offlineView) failure() (landing bool, reason string, ok bool) {

+// The roles a failed step can be: the sealer sealing, the target landing,

+// or a source box cleaning up after the open.

+const (

+ failSealer = "sealer"

+ failTarget = "target"

+ failSource = "source"

+// failure says whether a box has failed its step, which role it was and why:

+// the items the role's report failed, else what its failed report says, else

+// the word failed. Before the open it is the target landing, else the sealer

+// sealing (both at once go by the state the move is in). Once the move is

+// open or finishing a failed report of any role counts, source boxes included;

+// while it reverts only a failed source box does, since a target or sealer

+// that failed earlier is being put back.

+func (v offlineView) failure() (kind, reason string, ok bool) {

+ switch v.State {

+ case provision.OfflineOpen, provision.OfflineFinishing, provision.OfflineReverting:

+ roles := []string{moves.OfflineRoleSealer, moves.OfflineRoleTarget, moves.OfflineRoleSource}

+ if v.State == provision.OfflineReverting {

+ roles = roles[2:]

+ }

+ for _, role := range roles {

+ if r := v.failedReport(role); r != nil {

+ return role, r.reason(), true

+ }

+ return "", "", false

+ }

sealing := v.State == provision.OfflineStopping || v.State == provision.OfflineSealing || v.State == provision.OfflineSealed

sealReason, landReason := v.failed(moves.OfflineOwnerSeal), v.failed(moves.OfflineOwnerLanding)

sealRep, landRep := v.failedReport(moves.OfflineRoleSealer), v.failedReport(moves.OfflineRoleTarget)

@@ -1125,30 +1149,42 @@ func (v offlineView) failure() (landing bool, reason string, ok bool) {

if sealReason == "" && sealRep != nil {

sealReason = sealRep.reason()

}

- return false, sealReason, true

+ return failSealer, sealReason, true

case landFailed:

if landReason == "" && landRep != nil {

landReason = landRep.reason()

}

- return true, landReason, true

+ return failTarget, landReason, true

case v.Failed:

- return !sealing, moves.OfflinePhaseFailed, true

+ if sealing {

+ return failSealer, moves.OfflinePhaseFailed, true

+ }

+ return failTarget, moves.OfflinePhaseFailed, true

}

- return false, "", false

+ return "", "", false

}

-// sayFailure says a failed step: MV-O6 for the target, MV-O9 for the sealer.

+// sayFailure says a failed step: MV-O6 for the target, MV-O9 for the sealer,

+// MV-O65 for a source box.

func (w *offlineRun) sayFailure(v offlineView) bool {

- landing, reason, ok := v.failure()

+ kind, reason, ok := v.failure()

if !ok {

return false

}

host := w.m.host

- if landing {

+ switch kind {

+ case failTarget:

moveSay(w.m.out, fmt.Sprintf("%s could not take %s: %s. Run carlos move offline abort --host %s to move it back.", v.To, host, reason, host))

- return true

+ case failSource:

+ for _, r := range v.Reports {

+ if r.Role == moves.OfflineRoleSource && r.Phase == moves.OfflinePhaseFailed {

+ moveSay(w.m.out, fmt.Sprintf("%s could not clean up %s: %s. Run carlos move offline status --host %s for each item.", r.Box, host, reason, host))

+ return true

+ }

+ default:

+ moveSay(w.m.out, fmt.Sprintf("The move could not finish a step: %s. Run carlos move offline status --host %s for each item.", reason, host))

}

- moveSay(w.m.out, fmt.Sprintf("The move could not finish a step: %s. Run carlos move offline status --host %s for each item.", reason, host))

return true

}

@@ -1323,11 +1359,8 @@ func (w *offlineRun) sayServed(v offlineView) {

func (w *offlineRun) headline(r offlineReply) {

v, host := r.v, w.m.host

moveSay(w.m.out, moves.MoveStartingLine(host, v.From, v.To))

- switch v.State {

- case provision.OfflineStopping, provision.OfflineSealing, provision.OfflineSealed, provision.OfflineLanding, provision.OfflineLanded:

- if w.sayFailure(v) {

- return

- }

+ if w.sayFailure(v) {

+ return

}

switch v.State {

case provision.OfflineStopping:

@@ -1434,7 +1467,7 @@ func (w *offlineRun) runStatus(ctx context.Context) int {

for _, l := range v.Left {

moveSay(w.m.out, fmt.Sprintf("%s left %s in place: %s.", l.Holder, l.Where, l.Because))

}

- if failed || v.Failed {

+ if _, _, stepFailed := v.failure(); failed || v.Failed || stepFailed {

return moveExitWaiting

}

return moveExitDone

@@ -1523,6 +1556,9 @@ func (w *offlineRun) runOpen(ctx context.Context) int {

// next step is finish), not when the door only accepted the step.

return w.follow(ctx, r, func(r offlineReply) (int, bool) {

if r.v.State == provision.OfflineOpen {

+ if w.sayFailure(r.v) {

+ return moveExitWaiting, true

+ }

if w.served(r.v) {

w.sayServed(r.v)

} else {

@@ -1531,6 +1567,9 @@ func (w *offlineRun) runOpen(ctx context.Context) int {

if r.v.Next == "finish" {

return moveExitDone, true

}

+ if w.sayFailure(r.v) {

+ return moveExitWaiting, true

+ }

w.sayCleaning(r.v)

return 0, false

}

@@ -1561,6 +1600,9 @@ func (w *offlineRun) runFinish(ctx context.Context) int {

cur = r

return -1, true

}

+ if w.sayFailure(r.v) {

+ return moveExitWaiting, true

+ }

w.sayCleaning(r.v)

return 0, false

}

diff --git a/move_test.go b/move_test.go

index e96b4c8e1..6d34082fb 100644

--- a/move_test.go

+++ b/move_test.go

@@ -1755,3 +1755,53 @@ func TestMoveOfflineStatusShowsPathsLeft(t *testing.T) {

}

wantLines(t, out, "ie-box left /opt/carlos/live/acme.go.tito.io in place: a root-owned directory.")

}

+// A failed report seen after the open is not silent: status says it and exits

+// 4, and open stops polling with the same line.

+func TestMoveOfflineFailedReportAfterTheOpen(t *testing.T) {

+ failedRep := func(box, role, check string) frame {

+ f := offReport(box, role, "failed", true)

+ f["items"] = frame{check: frame{"ok": false, "reason": "the " + check + " check failed"}}

+ return f

+ }

+ for _, c := range []struct {

+ name string

+ state string

+ rep frame

+ say string

+ }{

+ {"target, open", "offline-open", failedRep("uk-box", "target", "config"),

+ "uk could not take acme.go.tito.io: the config check failed. Run carlos move offline abort --host acme.go.tito.io to move it back."},

+ {"sealer, finishing", "offline-finishing", failedRep("ie-box", "sealer", "lease"),

+ "The move could not finish a step: the lease check failed. Run carlos move offline status --host acme.go.tito.io for each item."},

+ {"source, open", "offline-open", failedRep("ie-box", "source", "warm-cache"),

+ "ie-box could not clean up acme.go.tito.io: the warm-cache check failed. Run carlos move offline status --host acme.go.tito.io for each item."},

+ {"source, reverting", "offline-reverting", failedRep("ie-box", "source", "warm-cache"),

+ "ie-box could not clean up acme.go.tito.io: the warm-cache check failed. Run carlos move offline status --host acme.go.tito.io for each item."},

+ } {

+ t.Run(c.name, func(t *testing.T) {

+ f := &fakeOfflineConsole{gets: []frame{offView(c.state, frame{"reports": []frame{c.rep}})}}

+ code, out := runOfflineAgainst(t, f, "status")

+ if code != 4 || !strings.HasPrefix(out, oStarting+"\n"+c.say+"\n") {

+ t.Errorf("status exit %d, output:\n%s\nwant 4 and %q after the headline", code, out, c.say)

+ }

+ if c.state == "offline-open" {

+ f := &fakeOfflineConsole{

+ posts: map[string][]offPost{"/open": {{202, offView("offline-open", nil)}}},

+ gets: []frame{offView(c.state, frame{"reports": []frame{c.rep}})},

+ }

+ code, out := runOfflineAgainst(t, f, "open")

+ if code != 4 || !strings.HasSuffix(out, c.say+"\n") {

+ t.Errorf("open exit %d, output:\n%s\nwant 4 ending in %q", code, out, c.say)

+ }

+ })

+ }

+ // A target or sealer that failed before an abort is being put back: a

+ // reverting move shows only a failed source box.

+ stale := offReport("uk-box", "target", "failed", true)

+ f := &fakeOfflineConsole{gets: []frame{offView("offline-reverting", frame{"next": "finish", "reports": []frame{stale}})}}

+ if code, out := runOfflineAgainst(t, f, "status"); code != 0 || strings.Contains(out, "could not") {

+ t.Errorf("a reverting move with an old failed target: exit %d:\n%s", code, out)

+ }