From fc00488e86c50ab0793f461981f3c14a14a56e34 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 15:20:20 -0400 Subject: [PATCH 1/7] feat(app): manage paired computers through appwire Add devices.list, devices.rename and devices.remove behind devices.manage, with typed schema shapes and devices.changed snapshots. List this computer first, preserve removed rows and omit unknown presence. Reuse local-first rename and existing removal receipts. Answer quiet, off and unavailable homes locally. Observe existing directory requests and watch frames without starting polling, sockets or background relay reads. Add in-process app-backend and wire-contract tests for paired homes, presence, local modes, refusal codes and retained mutation results. Tests were written but not run, as requested. No build or install was run. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/codeaf/app.go | 8 + cmd/codeaf/appdevices.go | 231 ++++++++++++++ cmd/codeaf/appdevices_test.go | 388 +++++++++++++++++++++++ cmd/codeaf/devices.go | 6 +- cmd/codeaf/devrename.go | 29 +- docs/changes/unreleased/0-app-devices.md | 22 ++ internal/appwire/devices.go | 116 +++++++ internal/appwire/devices_test.go | 72 +++++ internal/appwire/events.go | 1 + internal/appwire/server.go | 1 + internal/chatlist/copy.go | 5 + internal/syncsetup/devices.go | 280 ++++++++++++++++ internal/syncsetup/syncsetup.go | 2 +- 13 files changed, 1129 insertions(+), 32 deletions(-) create mode 100644 cmd/codeaf/appdevices.go create mode 100644 cmd/codeaf/appdevices_test.go create mode 100644 docs/changes/unreleased/0-app-devices.md create mode 100644 internal/appwire/devices.go create mode 100644 internal/appwire/devices_test.go create mode 100644 internal/syncsetup/devices.go diff --git a/cmd/codeaf/app.go b/cmd/codeaf/app.go index 287ed92590..1e32bf7266 100644 --- a/cmd/codeaf/app.go +++ b/cmd/codeaf/app.go @@ -446,6 +446,10 @@ type appBackend struct { drives *driveBook sync appSync syncPending map[string][]appSyncFact + + // devices observes existing sync traffic without starting a roster watch. + devices *appDevices + // organizeProposer supplies a naming door when the backend is embedded without held conversations. organizeProposer session.TeamProposer @@ -841,6 +845,7 @@ func newAppBackend(emit appwire.Emitter, open appOpener) *appBackend { mint: appMint, } b.emit = b.newAppInbox(emit) + b.devices = b.listenDevices() b.stopPhases = b.listenPhases() b.stopProgress = b.listenProgress() guard.Go("app/sweep", b.sweep) @@ -3936,6 +3941,9 @@ func (b *appBackend) Close() error { if already { return nil } + if b.devices != nil { + b.devices.close() + } // THIS MACHINE'S FACTORY IS LET GO BEFORE ANYTHING THAT WAITS ON A // CONVERSATION. Stopping the close watch waits for a carry-on that may be // opening a conversation, and nothing below it is the factory's business; diff --git a/cmd/codeaf/appdevices.go b/cmd/codeaf/appdevices.go new file mode 100644 index 0000000000..94488edf5f --- /dev/null +++ b/cmd/codeaf/appdevices.go @@ -0,0 +1,231 @@ +package main + +import ( + "cmp" + "context" + "errors" + "runtime" + "slices" + "sync" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/chatlist" + "github.com/Agent-Field/codeaf/internal/devname" + "github.com/Agent-Field/codeaf/internal/directory" + "github.com/Agent-Field/codeaf/internal/dirwatch" + "github.com/Agent-Field/codeaf/internal/guard" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/identity" + "github.com/Agent-Field/codeaf/internal/pair" + "github.com/Agent-Field/codeaf/internal/syncsetup" + "github.com/Agent-Field/codeaf/internal/wireauth" +) + +// appDevices observes traffic already owned by the engine. Opening Settings +// does not start a follower; only a person's list request reads the relay. +type appDevices struct { + home string + watch *syncsetup.DeviceObserver + op sync.Mutex + done chan struct{} +} + +func (b *appBackend) listenDevices() *appDevices { + d := &appDevices{home: home.Dir(), done: make(chan struct{})} + d.watch = syncsetup.ObserveDevices(d.home) + guard.Go("app/devices", func() { + defer close(d.done) + for range d.watch.Changes() { + if view, err := d.snapshot(); err == nil { + b.emit.Emit(appwire.EventDevicesChanged, appwire.DevicesResult{Devices: view}) + } + } + }) + return d +} + +func (d *appDevices) close() { + d.watch.Close() + <-d.done +} + +func (d *appDevices) snapshot() (appwire.Devices, error) { + // A first read needs a stable local device id without turning a fresh + // identity into a fleet that is allowed to contact the relay. + id, err := identity.EnsureSolo(d.home) + if err != nil { + return appwire.Devices{}, err + } + self, err := identity.Device(d.home) + if err != nil { + return appwire.Devices{}, err + } + view := appwire.Devices{Mode: appwire.DevicesOn} + known := d.watch.Snapshot() + switch { + case !syncsetup.Resolve(d.home).On(): + view.Mode, view.Line = appwire.DevicesOff, chatlist.SyncOff + case !cell.Available(): + view.Mode, view.Line = appwire.DevicesUnavailable, cell.UnavailableReason() + case errors.Is(known.Refused, dirwatch.ErrRevoked): + view.Mode, view.Line = appwire.DevicesUnavailable, chatlist.Removed + case errors.Is(known.Refused, dirwatch.ErrRotated): + view.Mode, view.Line = appwire.DevicesUnavailable, chatlist.Replaced + case !syncsetup.MayTalk(d.home): + view.Mode, view.Line = appwire.DevicesQuiet, chatlist.DevicesQuiet + } + if view.Mode != appwire.DevicesOn { + if rec, ok := known.Devices[self.ID()]; ok { + known.Devices = map[string]directory.Device{self.ID(): rec} + } else { + known.Devices = nil + } + known.Online = nil + } + open := func(sealed string) (string, error) { + return directory.OpenName(directory.MetadataKey(id.CellKey()), sealed) + } + view.Devices = make([]appwire.Device, 0, len(known.Devices)+1) + for deviceID, rec := range known.Devices { + row := appwire.Device{ID: deviceID, Name: chatlist.DeviceName(known.Devices, deviceID, open), Platform: rec.Platform, Self: deviceID == self.ID(), Removed: rec.Revoked} + if rec.LastSeen > 0 { + row.LastSeen = time.UnixMilli(rec.LastSeen).UTC() + } + if online, ok := known.Online[deviceID]; ok && !rec.Revoked { + row.Online = &online + } + view.Devices = append(view.Devices, row) + } + selfAt := slices.IndexFunc(view.Devices, func(row appwire.Device) bool { return row.Self }) + if selfAt < 0 { + view.Devices = append(view.Devices, appwire.Device{ID: self.ID(), Self: true}) + selfAt = len(view.Devices) - 1 + } + view.Devices[selfAt].Name, view.Devices[selfAt].Platform = devname.Name(d.home), runtime.GOOS + slices.SortFunc(view.Devices, func(a, b appwire.Device) int { + return cmp.Or(-cmp.Compare(rank(a.Self), rank(b.Self)), cmp.Compare(rank(a.Removed), rank(b.Removed)), cmp.Compare(a.Name, b.Name), cmp.Compare(a.ID, b.ID)) + }) + return view, nil +} + +func (b *appBackend) ListDevices() (appwire.Devices, error) { + d := b.devices + d.op.Lock() + defer d.op.Unlock() + view, err := d.snapshot() + if err != nil || view.Mode != appwire.DevicesOn { + return view, err + } + s, ok, err := syncsetup.Open(d.home) + if err != nil || !ok { + view.Devices, view.Mode = view.Devices[:1], appwire.DevicesUnavailable + view.Devices[0].Online = nil + view.Line = chatlist.SyncOff + if err != nil { + view.Line = err.Error() + } + return view, nil + } + ctx, cancel := context.WithTimeout(context.Background(), syncsetup.DeviceRequestWithin) + defer cancel() + if _, err := s.Dir.List(ctx); err != nil { + return appwire.Devices{}, deviceError(err) + } + if _, err := s.Dir.Presence(ctx); err != nil && !errors.Is(err, directory.ErrTooOld) { + return appwire.Devices{}, deviceError(err) + } + return d.snapshot() +} + +func (b *appBackend) RenameDevice(typed string) (appwire.DeviceRenameResult, error) { + d := b.devices + d.op.Lock() + defer d.op.Unlock() + if _, err := devname.Clean(typed); err != nil { + return appwire.DeviceRenameResult{}, appwire.Errorf(appwire.CodeBadParams, "%s", err) + } + if reason := cell.UnavailableReason(); reason != "" { + return appwire.DeviceRenameResult{}, appwire.Errorf(appwire.CodeEngine, "%s", reason) + } + line, err := syncsetup.RenameDevice(context.Background(), d.home, typed) + if err != nil { + return appwire.DeviceRenameResult{}, err + } + view, err := d.snapshot() + if err != nil { + return appwire.DeviceRenameResult{}, err + } + b.emit.Emit(appwire.EventDevicesChanged, appwire.DevicesResult{Devices: view}) + return appwire.DeviceRenameResult{Device: view.Devices[0], Line: line}, nil +} + +func (b *appBackend) RemoveDevice(deviceID string) (appwire.DeviceRemoveResult, error) { + d := b.devices + d.op.Lock() + defer d.op.Unlock() + view, err := d.snapshot() + if err != nil { + return appwire.DeviceRemoveResult{}, err + } + if deviceID == view.Devices[0].ID { + return appwire.DeviceRemoveResult{}, appwire.Errorf(appwire.CodeBadParams, "%s", chatlist.RemoveSelf) + } + if view.Mode != appwire.DevicesOn { + return appwire.DeviceRemoveResult{}, appwire.Errorf(appwire.CodeNotFound, "%s", chatlist.DeviceMissing) + } + s, ok, err := syncsetup.Open(d.home) + if err != nil { + return appwire.DeviceRemoveResult{}, deviceError(err) + } + if !ok { + return appwire.DeviceRemoveResult{}, appwire.Errorf(appwire.CodeEngine, "%s", chatlist.SyncOff) + } + ctx, cancel := context.WithTimeout(context.Background(), syncsetup.DeviceRequestWithin) + defer cancel() + listing, err := s.Dir.List(ctx) + if err != nil { + return appwire.DeviceRemoveResult{}, deviceError(err) + } + rec, exists := listing.Devices[deviceID] + if !exists { + return appwire.DeviceRemoveResult{}, deviceError(directory.ErrNotFound) + } + open := func(sealed string) (string, error) { + return directory.OpenName(directory.MetadataKey(s.Identity.CellKey()), sealed) + } + name := chatlist.DeviceName(listing.Devices, deviceID, open) + line := name + " is already removed." + if !rec.Revoked { + if err := s.Dir.Revoke(ctx, deviceID); err != nil { + return appwire.DeviceRemoveResult{}, deviceError(err) + } + line = pair.ChatsRevokedLine(name) + } + // Success is retained even if a later user-requested refresh cannot reach + // the relay. The revoke already updated this process's remembered record. + view, err = d.snapshot() + if err == nil { + b.emit.Emit(appwire.EventDevicesChanged, appwire.DevicesResult{Devices: view}) + } + return appwire.DeviceRemoveResult{ID: deviceID, Line: line}, nil +} + +func deviceError(err error) error { + switch { + case errors.Is(err, directory.ErrSelfRevoke): + return appwire.Errorf(appwire.CodeBadParams, "%s", chatlist.RemoveSelf) + case errors.Is(err, directory.ErrNotFound): + return appwire.Errorf(appwire.CodeNotFound, "%s", chatlist.DeviceMissing) + case errors.Is(err, directory.ErrUnreachable): + return appwire.Errorf(appwire.CodeUnreachable, "%s", chatlist.Unreachable) + case errors.Is(err, directory.ErrRevoked): + return appwire.Errorf(appwire.CodeEngine, "%s", chatlist.Removed) + case errors.Is(err, directory.ErrRotated): + return appwire.Errorf(appwire.CodeEngine, "%s", chatlist.Replaced) + case errors.Is(err, wireauth.ErrSkew): + return appwire.Errorf(appwire.CodeEngine, "%s", chatlist.ClockOff) + } + return err +} diff --git a/cmd/codeaf/appdevices_test.go b/cmd/codeaf/appdevices_test.go new file mode 100644 index 0000000000..947dd7371f --- /dev/null +++ b/cmd/codeaf/appdevices_test.go @@ -0,0 +1,388 @@ +package main + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "runtime" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cell/celltest" + "github.com/Agent-Field/codeaf/internal/chatlist" + "github.com/Agent-Field/codeaf/internal/devname" + "github.com/Agent-Field/codeaf/internal/directory" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/identity" + "github.com/Agent-Field/codeaf/internal/pair" + "github.com/Agent-Field/codeaf/internal/reqsign" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +// The same signed in-process directory as syncsetup's relay fixture, without +// a chat engine or a provider. Every request, including watch upgrades, counts. +func appDevicesRelay(t *testing.T, wrap ...func(http.Handler) http.Handler) (*httptest.Server, *atomic.Int64, *atomic.Bool) { + t.Helper() + var mu sync.Mutex + dirs := map[string]directory.Directory{} + open := func(id string) (directory.Directory, error) { + mu.Lock() + defer mu.Unlock() + if dirs[id] == nil { + dirs[id] = directory.NewMemory(time.Now) + } + return dirs[id], nil + } + hits, down := &atomic.Int64{}, &atomic.Bool{} + door := directory.Handler(reqsign.AuthenticateAt(time.Now), open) + for _, one := range wrap { + door = one(door) + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + if down.Load() { + http.Error(w, "offline", http.StatusServiceUnavailable) + return + } + door.ServeHTTP(w, r) + })) + t.Cleanup(srv.Close) + t.Setenv(syncsetup.URLVar, srv.URL) + t.Setenv(syncsetup.IntervalVar, "") + return srv, hits, down +} + +func appDeviceSync(t *testing.T, dir string) *syncsetup.Sync { + t.Helper() + s, ok, err := syncsetup.Open(dir) + if err != nil || !ok { + t.Fatalf("open sync: %v, %v", ok, err) + } + return s +} + +func appDeviceBackend(t *testing.T, dir string) (*appBackend, *appRecorder, *actionWire) { + t.Helper() + t.Setenv(home.EnvVar, dir) + rec := newAppRecorder() + b := newAppBackend(rec, nil) + // This fixture exercises device management only; suppress the unrelated + // process warmer that normally follows hello. + b.warmed = make(chan struct{}) + close(b.warmed) + t.Cleanup(func() { _ = b.Close() }) + w := newActionWire(t, b) + if !slices.Contains(w.hello, appwire.FeatureDevicesManage) { + t.Fatal("hello is missing devices.manage") + } + return b, rec, w +} + +func appDeviceList(t *testing.T, w *actionWire) appwire.Devices { + t.Helper() + return machineCall[appwire.DevicesResult](t, w, "devices.list", struct{}{}).Devices +} + +func appDeviceEvent(t *testing.T, rec *appRecorder, match func(appwire.Devices) bool) int { + t.Helper() + return rec.waitFor(t, 5*time.Second, func(event string, data any) bool { + return event == appwire.EventDevicesChanged && match(data.(appwire.DevicesResult).Devices) + }) +} + +// V1, V5: real watch traffic updates presence without any roster/presence read. +func TestAppDevicesV1V5ExistingWatchSuppliesPresence(t *testing.T) { + celltest.Available(t) + _, hits, _ := appDevicesRelay(t) + a, b := t.TempDir(), t.TempDir() + id, err := identity.Ensure(a) + if err != nil { + t.Fatal(err) + } + if _, err := identity.Adopt(b, id, false); err != nil { + t.Fatal(err) + } + sa, sb := appDeviceSync(t, a), appDeviceSync(t, b) + for _, s := range []*syncsetup.Sync{sa, sb} { + if err := s.Rename(context.Background(), "computer"); err != nil { + t.Fatal(err) + } + } + _, rec, w := appDeviceBackend(t, a) + view := appDeviceList(t, w) + if view.Devices[1].Online == nil || *view.Devices[1].Online { + t.Fatalf("known offline presence: %+v", view) + } + before := hits.Load() + // These sockets belong to other engine work, not to the Devices adapter. + follow := sa.Follow() + if follow == nil { + t.Fatal("the real directory has no follower") + } + t.Cleanup(follow.Close) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + for !follow.State().Up { + select { + case <-follow.Changes(): + case <-ctx.Done(): + t.Fatal("the existing follower did not open") + } + } + remote, err := sb.Dir.(directory.Watcher).Watch(ctx) + if err != nil { + t.Fatal(err) + } + t.Cleanup(remote.Close) + appDeviceEvent(t, rec, func(v appwire.Devices) bool { + return len(v.Devices) == 2 && v.Devices[1].Online != nil && *v.Devices[1].Online + }) + remote.Close() + appDeviceEvent(t, rec, func(v appwire.Devices) bool { + return len(v.Devices) == 2 && v.Devices[1].Online != nil && !*v.Devices[1].Online && !v.Devices[1].LastSeen.IsZero() + }) + rec.mu.Lock() + after := len(rec.events) + rec.mu.Unlock() + follow.Close() + seen := 0 + rec.waitFor(t, 5*time.Second, func(event string, data any) bool { + i := seen + seen++ + if i < after || event != appwire.EventDevicesChanged { + return false + } + v := data.(appwire.DevicesResult).Devices + return len(v.Devices) == 2 && v.Devices[1].Online == nil + }) + if hits.Load()-before != 2 { + t.Fatalf("watch events caused %d requests beyond the two upgrades", hits.Load()-before) + } +} + +// V1: an older relay that cannot answer presence leaves it unknown. +func TestAppDevicesV1OldRelayPresenceIsUnknown(t *testing.T) { + celltest.Available(t) + _, _, _ = appDevicesRelay(t, func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v1/dir/presence" { + http.NotFound(w, r) + return + } + next.ServeHTTP(w, r) + }) + }) + dir := t.TempDir() + if _, err := identity.Ensure(dir); err != nil { + t.Fatal(err) + } + s := appDeviceSync(t, dir) + if err := s.Rename(context.Background(), "desk"); err != nil { + t.Fatal(err) + } + _, _, w := appDeviceBackend(t, dir) + view := appDeviceList(t, w) + if view.Mode != appwire.DevicesOn || len(view.Devices) != 1 || view.Devices[0].Online != nil { + t.Fatalf("old relay presence was invented: %+v", view) + } +} + +// V1-V3: two paired homes use the real backend and registered wire methods. +func TestAppDevicesV1V2V3PairedHomes(t *testing.T) { + celltest.Available(t) + srv, hits, _ := appDevicesRelay(t) + a, b := t.TempDir(), t.TempDir() + id, err := identity.Ensure(a) + if err != nil { + t.Fatal(err) + } + if _, err := identity.Adopt(b, id, false); err != nil { + t.Fatal(err) + } + sa, sb := appDeviceSync(t, a), appDeviceSync(t, b) + for dir, name := range map[string]string{a: "desk", b: "laptop"} { + if _, err := devname.Set(dir, name); err != nil { + t.Fatal(err) + } + if err := appDeviceSync(t, dir).Rename(context.Background(), name); err != nil { + t.Fatal(err) + } + } + backend, rec, wa := appDeviceBackend(t, a) + _, _, wb := appDeviceBackend(t, b) + view := appDeviceList(t, wa) + if view.Mode != appwire.DevicesOn || len(view.Devices) != 2 || view.Devices[0].ID != sa.Device.ID() || !view.Devices[0].Self || view.Devices[0].Name != "desk" || view.Devices[1].Name != "laptop" || view.Devices[1].Self || view.Devices[1].Platform != runtime.GOOS || view.Devices[1].Removed { + t.Fatalf("paired list: %+v", view) + } + for _, params := range []map[string]any{{"name": ""}, {"name": strings.Repeat("x", devname.Most+1)}, {"name": "a\nb"}, {"name": "other", "id": sb.Device.ID()}} { + if _, err := wa.call("devices.rename", params); err == nil || err.Code != appwire.CodeBadParams { + t.Fatalf("rename %v: %v", params, err) + } + } + renamed := machineCall[appwire.DeviceRenameResult](t, wa, "devices.rename", appwire.DeviceRenameParams{Name: " studio "}) + if renamed.Device.ID != sa.Device.ID() || renamed.Device.Name != "studio" || renamed.Line != pair.RenamedLine("studio") { + t.Fatalf("rename receipt: %+v", renamed) + } + appDeviceEvent(t, rec, func(v appwire.Devices) bool { return v.Devices[0].Name == "studio" }) + other := appDeviceList(t, wb) + if len(other.Devices) != 2 || other.Devices[0].ID != sb.Device.ID() || other.Devices[1].Name != "studio" { + t.Fatalf("the other home sees: %+v", other) + } + // The CLI's reader sees the same shared name, independently of app state. + rows, err := chatsKindOf(sb.Dir, id.CellKey(), sb.Device.ID(), sb.Relay).rows() + if err != nil || len(rows) != 2 || rows[1].Name != "studio" { + t.Fatalf("CLI roster: %+v, %v", rows, err) + } + before := hits.Load() + if _, err := wa.call("devices.remove", appwire.IDParams{ID: sa.Device.ID()}); err == nil || err.Code != appwire.CodeBadParams { + t.Fatalf("remove self: %v", err) + } + if hits.Load() != before { + t.Fatal("self removal reached the relay") + } + if _, err := wa.call("devices.remove", appwire.IDParams{ID: "unknown"}); err == nil || err.Code != appwire.CodeNotFound { + t.Fatalf("remove unknown: %v", err) + } + removed := machineCall[appwire.DeviceRemoveResult](t, wa, "devices.remove", appwire.IDParams{ID: sb.Device.ID()}) + if removed.ID != sb.Device.ID() || removed.Line != pair.ChatsRevokedLine("laptop") { + t.Fatalf("remove receipt: %+v", removed) + } + appDeviceEvent(t, rec, func(v appwire.Devices) bool { return len(v.Devices) == 2 && v.Devices[1].Removed }) + view = appDeviceList(t, wa) + if len(view.Devices) != 2 || !view.Devices[1].Removed { + t.Fatalf("removed device disappeared: %+v", view) + } + if _, err := wb.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeEngine || err.Message != chatlist.Removed { + t.Fatalf("the removed computer's refusal: %v", err) + } + before = hits.Load() + unavailable := appDeviceList(t, wb) + if unavailable.Mode != appwire.DevicesUnavailable || unavailable.Line != chatlist.Removed || len(unavailable.Devices) != 1 || !unavailable.Devices[0].Removed || hits.Load() != before { + t.Fatalf("known removal was not answered locally: %+v", unavailable) + } + srv.Close() + if _, err := wa.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeUnreachable { + t.Fatalf("refresh after removal: %v", err) + } + retained, err := backend.devices.snapshot() + if err != nil || len(retained.Devices) != 2 || !retained.Devices[1].Removed { + t.Fatalf("failed refresh undid removal: %+v, %v", retained, err) + } +} + +// V4-V5: local reads, local rename and self refusal leave quiet/off homes quiet. +func TestAppDevicesV4V5LocalModesMakeNoRelayRequests(t *testing.T) { + for _, mode := range []string{appwire.DevicesQuiet, appwire.DevicesOff, appwire.DevicesUnavailable} { + t.Run(mode, func(t *testing.T) { + celltest.Available(t) + srv, hits, _ := appDevicesRelay(t) + dir := t.TempDir() + if mode == appwire.DevicesOff { + // An off home that had paired must also stay local. + if _, err := identity.Ensure(dir); err != nil { + t.Fatal(err) + } + if err := syncsetup.SaveRelayURL(dir, srv.URL); err != nil { + t.Fatal(err) + } + t.Setenv(syncsetup.URLVar, "off") + } + if mode == appwire.DevicesUnavailable { + if _, err := identity.Ensure(dir); err != nil { + t.Fatal(err) + } + t.Cleanup(cell.UseAvailabilityInTests(false)) + } + backend, rec, w := appDeviceBackend(t, dir) + view := appDeviceList(t, w) + if view.Mode != mode || len(view.Devices) != 1 || !view.Devices[0].Self || view.Devices[0].Online != nil || !view.Devices[0].LastSeen.IsZero() || view.Line == "" { + t.Fatalf("local list: %+v", view) + } + if mode == appwire.DevicesQuiet && (view.Line != chatlist.DevicesQuiet || !strings.Contains(view.Line, "codeaf pair") || !identity.Solo(dir)) { + t.Fatalf("fresh home is not quiet with terminal instructions: %+v", view) + } + if mode == appwire.DevicesOff && view.Line != chatlist.SyncOff { + t.Fatalf("off copy: %q", view.Line) + } + if mode != appwire.DevicesUnavailable { + renamed := machineCall[appwire.DeviceRenameResult](t, w, "devices.rename", appwire.DeviceRenameParams{Name: "kitchen"}) + if renamed.Line != pair.RenamedAloneLine("kitchen") || renamed.Device.ID != view.Devices[0].ID { + t.Fatalf("local rename: %+v", renamed) + } + appDeviceEvent(t, rec, func(v appwire.Devices) bool { return v.Devices[0].Name == "kitchen" }) + } + if _, err := w.call("devices.remove", appwire.IDParams{ID: view.Devices[0].ID}); err == nil || err.Code != appwire.CodeBadParams { + t.Fatalf("local self refusal: %v", err) + } + _ = appDeviceList(t, w) + if err := backend.Close(); err != nil { + t.Fatal(err) + } + if hits.Load() != 0 { + t.Fatalf("local home sent %d relay requests", hits.Load()) + } + }) + } +} + +// V5: while Devices was never opened, engine-owned writes emit without a read. +func TestAppDevicesV5PassiveEventsDoNotReadRelay(t *testing.T) { + celltest.Available(t) + _, hits, _ := appDevicesRelay(t) + dir := t.TempDir() + if _, err := identity.Ensure(dir); err != nil { + t.Fatal(err) + } + s := appDeviceSync(t, dir) + before := hits.Load() + _, rec, _ := appDeviceBackend(t, dir) + if err := s.Rename(context.Background(), "elsewhere in the engine"); err != nil { + t.Fatal(err) + } + // Sync.Rename publishes a directory record. A CLI rename also saves the + // chosen local name, as it does here before a second engine-owned write. + if _, err := devname.Set(dir, "window name"); err != nil { + t.Fatal(err) + } + if err := s.Rename(context.Background(), "window name"); err != nil { + t.Fatal(err) + } + appDeviceEvent(t, rec, func(v appwire.Devices) bool { return v.Devices[0].Name == "window name" }) + if hits.Load()-before != 2 { + t.Fatalf("passive events sent %d requests beyond the two writes", hits.Load()-before) + } +} + +// V2 and the wire error contract: an unreachable relay keeps the local name. +func TestAppDevicesV2OfflineRenameAndUnreachableList(t *testing.T) { + celltest.Available(t) + srv, _, _ := appDevicesRelay(t) + dir := t.TempDir() + if _, err := identity.Ensure(dir); err != nil { + t.Fatal(err) + } + _, rec, w := appDeviceBackend(t, dir) + srv.Close() + if _, err := w.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeUnreachable || err.Message != chatlist.Unreachable { + t.Fatalf("unreachable list: %v", err) + } + raw, err := w.call("devices.rename", appwire.DeviceRenameParams{Name: "offline desk"}) + if err != nil { + t.Fatal(err) + } + var got appwire.DeviceRenameResult + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatal(err) + } + if got.Device.Name != "offline desk" || got.Line != pair.RenamedHereLine("offline desk") || devname.Name(dir) != "offline desk" { + t.Fatalf("offline rename: %+v", got) + } + appDeviceEvent(t, rec, func(v appwire.Devices) bool { return v.Devices[0].Name == "offline desk" }) +} diff --git a/cmd/codeaf/devices.go b/cmd/codeaf/devices.go index 682e94662d..f0e3ed0201 100644 --- a/cmd/codeaf/devices.go +++ b/cmd/codeaf/devices.go @@ -313,7 +313,7 @@ type chatsRow struct { // listRequestWithin bounds one request to the relay so a silent one cannot hang // the command. -const listRequestWithin = 20 * time.Second +const listRequestWithin = syncsetup.DeviceRequestWithin func (c chatsKind) rows() ([]chatsRow, error) { ctx, cancel := context.WithTimeout(context.Background(), listRequestWithin) @@ -393,7 +393,7 @@ func (c chatsKind) stop(name string) (string, error) { func (c chatsKind) revoke(row chatsRow) (string, error) { if row.This { - return "", errors.New("that is this device — remove it from another of your devices instead") + return "", errors.New(chatlist.RemoveSelf) } ctx, cancel := context.WithTimeout(context.Background(), listRequestWithin) defer cancel() @@ -413,7 +413,7 @@ func (c chatsKind) explain(err error) error { case errors.Is(err, directory.ErrUnreachable): return fmt.Errorf("the sync service at %s cannot be reached from here — check this computer's network", c.relay) case errors.Is(err, directory.ErrNotFound): - return errors.New("that device is not in your list any more — `codeaf devices` shows the ones there are") + return errors.New(chatlist.DeviceMissing) } return err } diff --git a/cmd/codeaf/devrename.go b/cmd/codeaf/devrename.go index a85582aa91..b6346bd286 100644 --- a/cmd/codeaf/devrename.go +++ b/cmd/codeaf/devrename.go @@ -17,44 +17,17 @@ import ( "errors" "fmt" "strings" - "time" "github.com/Agent-Field/codeaf/internal/cell" "github.com/Agent-Field/codeaf/internal/devname" "github.com/Agent-Field/codeaf/internal/home" - "github.com/Agent-Field/codeaf/internal/pair" "github.com/Agent-Field/codeaf/internal/syncsetup" ) -// renameWithin bounds the one request a rename makes, so a silent relay cannot -// hang the command. -const renameWithin = 20 * time.Second - // renameThisDevice keeps typed as the name of the computer whose home is dir, // tells the fleet when it may, and returns the sentence a person reads. func renameThisDevice(ctx context.Context, dir, typed string) (string, error) { - name, err := devname.Set(dir, typed) - if err != nil { - return "", err - } - if !syncsetup.MayTalk(dir) { - return pair.RenamedAloneLine(name), nil - } - if tellFleet(ctx, dir, name) != nil { - return pair.RenamedHereLine(name), nil - } - return pair.RenamedLine(name), nil -} - -// tellFleet puts the new name in the directory, and says why not when it cannot. -func tellFleet(ctx context.Context, dir, name string) error { - s, ok, err := syncsetup.Open(dir) - if err != nil || !ok { - return errors.Join(err, errors.New("sync is off")) - } - ctx, cancel := context.WithTimeout(ctx, renameWithin) - defer cancel() - return s.Rename(ctx, name) + return syncsetup.RenameDevice(ctx, dir, typed) } // runDevicesRename is `codeaf devices rename `. The name may be several diff --git a/docs/changes/unreleased/0-app-devices.md b/docs/changes/unreleased/0-app-devices.md new file mode 100644 index 0000000000..2e7ca2674f --- /dev/null +++ b/docs/changes/unreleased/0-app-devices.md @@ -0,0 +1,22 @@ +--- +kind: added +title: Desktop engines can list, rename and remove paired computers +pr: 0 +surface: [engine] +invalidates: + - "The app engine had no device-management methods. Engines that advertise devices.manage now serve devices.list, devices.rename and devices.remove." +--- + +The app wire lists this computer first and includes other computers paired to +the same identity, their platform, removal state, last sighting and known +presence. Unknown presence is omitted. Only this computer can be renamed; another +computer can be removed by its id. Receipts use the engine's existing rename and +removal sentences, including the limits of removing a lost computer. + +Never-paired homes list only this computer and explain how to add another with +`codeaf pair` from a terminal. Sync-off and unavailable homes also answer locally. +These reads make no relay request. Device management starts no polling or watch +socket: an explicit list reads the roster and presence, while `devices.changed` +reuses facts received by existing engine requests and sockets. Rename and remove +emit a snapshot without a follow-up relay read, preserving a completed change +when a later refresh fails. Pairing itself and the desktop Settings UI are separate. diff --git a/internal/appwire/devices.go b/internal/appwire/devices.go new file mode 100644 index 0000000000..b2651207dd --- /dev/null +++ b/internal/appwire/devices.go @@ -0,0 +1,116 @@ +package appwire + +import ( + "encoding/json" + "time" +) + +const ( + FeatureDevicesManage = "devices.manage" + EventDevicesChanged = "devices.changed" + DevicesQuiet = "quiet" + DevicesOn = "on" + DevicesOff = "off" + DevicesUnavailable = "unavailable" +) + +// Device is a computer paired to this identity. Unknown presence is absent. +type Device struct { + ID string `json:"id"` + Name string `json:"name"` + Platform string `json:"platform,omitempty"` + Self bool `json:"self"` + Removed bool `json:"removed"` + LastSeen time.Time `json:"last_seen,omitzero"` + Online *bool `json:"online,omitempty"` +} + +type Devices struct { + Devices []Device `json:"devices"` + Mode string `json:"mode" enum:"quiet,on,off,unavailable"` + Line string `json:"line,omitempty"` +} + +// DevicesResult also carries devices.changed, without a second snapshot shape. +type DevicesResult struct { + Devices Devices `json:"devices"` +} + +type DeviceRenameParams struct { + Name string `json:"name"` +} + +type DeviceListParams struct{} + +type DeviceRemoveParams struct { + ID string `json:"id"` +} + +func (p *DeviceListParams) UnmarshalJSON(raw []byte) error { + type plain DeviceListParams + return decodeStrict("devices.list", raw, (*plain)(p)) +} + +func (p *DeviceRenameParams) UnmarshalJSON(raw []byte) error { + type plain DeviceRenameParams + return decodeStrict("devices.rename", raw, (*plain)(p)) +} + +func (p *DeviceRemoveParams) UnmarshalJSON(raw []byte) error { + type plain DeviceRemoveParams + return decodeStrict("devices.remove", raw, (*plain)(p)) +} + +type DeviceRenameResult struct { + Device Device `json:"device"` + Line string `json:"line"` +} + +type DeviceRemoveResult struct { + ID string `json:"id"` + Line string `json:"line"` +} + +type DeviceManager interface { + ListDevices() (Devices, error) + RenameDevice(string) (DeviceRenameResult, error) + RemoveDevice(string) (DeviceRemoveResult, error) +} + +func withDevicesFeatures(hello Hello, e Engine) Hello { + if _, ok := e.(DeviceManager); ok { + return withFeature(hello, FeatureDevicesManage) + } + return hello +} + +func init() { + methods["devices.list"] = typed(func(e Engine, _ DeviceListParams) (*DevicesResult, error) { + door, ok := e.(DeviceManager) + if !ok { + return nil, Errorf(CodeUnknownMethod, "this engine does not manage computers") + } + got, err := door.ListDevices() + got.Devices = nonNil(got.Devices) + return &DevicesResult{Devices: got}, err + }) + methods["devices.rename"] = typed(func(e Engine, p DeviceRenameParams) (*DeviceRenameResult, error) { + door, ok := e.(DeviceManager) + if !ok { + return nil, Errorf(CodeUnknownMethod, "this engine does not manage computers") + } + got, err := door.RenameDevice(p.Name) + return &got, err + }) + methods["devices.remove"] = typed(func(e Engine, p DeviceRemoveParams) (*DeviceRemoveResult, error) { + door, ok := e.(DeviceManager) + if !ok { + return nil, Errorf(CodeUnknownMethod, "this engine does not manage computers") + } + if _, err := required("id", p.ID); err != nil { + return nil, err + } + got, err := door.RemoveDevice(p.ID) + return &got, err + }) +} diff --git a/internal/appwire/devices_test.go b/internal/appwire/devices_test.go new file mode 100644 index 0000000000..5a2aefcc30 --- /dev/null +++ b/internal/appwire/devices_test.go @@ -0,0 +1,72 @@ +package appwire + +import ( + "encoding/json" + "reflect" + "slices" + "testing" + "time" +) + +// V6: registration alone does not advertise device management. +func TestDevicesV6OldEngineHasNoFeatureOrMethods(t *testing.T) { + w := startWire(t, &fakeEngine{}) + if slices.ContainsFunc(features(w.next()), func(v any) bool { return v == FeatureDevicesManage }) { + t.Fatal("an old engine advertised device management") + } + for _, name := range []string{"devices.list", "devices.rename", "devices.remove"} { + if _, err := methods[name].call(&fakeEngine{}, json.RawMessage(`{}`)); codeOf(err) != CodeUnknownMethod { + t.Fatalf("%s: %v", name, err) + } + } +} + +// V1, V5: unknown facts are absent and the event has the same snapshot envelope as list. +func TestDevicesV1V5WireShapes(t *testing.T) { + device := Device{ID: "self", Name: "desk", Self: true} + raw, err := json.Marshal(DevicesResult{Devices: Devices{Devices: []Device{device}, Mode: DevicesQuiet, Line: "pair from a terminal"}}) + if err != nil { + t.Fatal(err) + } + want := `{"devices":{"devices":[{"id":"self","name":"desk","self":true,"removed":false}],"mode":"quiet","line":"pair from a terminal"}}` + if string(raw) != want { + t.Fatalf("list = %s, want %s", raw, want) + } + for _, reply := range []struct { + value any + want string + }{ + {DeviceRenameResult{Device: device, Line: "renamed"}, `{"device":{"id":"self","name":"desk","self":true,"removed":false},"line":"renamed"}`}, + {DeviceRemoveResult{ID: "other", Line: "removed"}, `{"id":"other","line":"removed"}`}, + } { + raw, err := json.Marshal(reply.value) + if err != nil || string(raw) != reply.want { + t.Fatalf("%T = %s, %v; want %s", reply.value, raw, err, reply.want) + } + } + offline := false + device.Online, device.LastSeen = &offline, time.Date(2026, 10, 26, 12, 0, 0, 0, time.UTC) + raw, err = json.Marshal(device) + if err != nil || string(raw) != `{"id":"self","name":"desk","self":true,"removed":false,"last_seen":"2026-10-26T12:00:00Z","online":false}` { + t.Fatalf("known facts = %s, %v", raw, err) + } + if EventShape(EventDevicesChanged) != reflect.TypeFor[DevicesResult]() { + t.Fatal("devices.changed is not the list envelope") + } + schema := Schema() + for _, name := range []string{"devices.list", "devices.rename", "devices.remove"} { + if !slices.Contains(Methods(), name) || schema["methods"].(map[string]any)[name] == nil { + t.Fatalf("missing method or schema: %s", name) + } + } + if schema["events"].(map[string]any)[EventDevicesChanged] == nil { + t.Fatal("missing event schema") + } + for name, want := range map[string]string{ + "devices.list": "DevicesResult", "devices.rename": "DeviceRenameResult", "devices.remove": "DeviceRemoveResult", + } { + if got := methods[name].result.Name(); got != want { + t.Fatalf("%s result = %s, want %s", name, got, want) + } + } +} diff --git a/internal/appwire/events.go b/internal/appwire/events.go index 8059bcd88c..f9a0b23105 100644 --- a/internal/appwire/events.go +++ b/internal/appwire/events.go @@ -45,6 +45,7 @@ var eventShapes = map[string]reflect.Type{ EventOrderAsking: reflect.TypeFor[OrderFired](), EventSettingsChanged: reflect.TypeFor[SettingsChanged](), EventProviderChanged: reflect.TypeFor[ProviderChanged](), + EventDevicesChanged: reflect.TypeFor[DevicesResult](), EventLog: reflect.TypeFor[Log](), EventCollectionChanged: reflect.TypeFor[CollectionChanged](), diff --git a/internal/appwire/server.go b/internal/appwire/server.go index 12d8b6cd51..ec5c1f6ffd 100644 --- a/internal/appwire/server.go +++ b/internal/appwire/server.go @@ -163,6 +163,7 @@ func (s *Server) Serve(ctx context.Context, in io.Reader, hello Hello, engine En hello = withProcessFeatures(hello, engine) hello = withProcessActionFeature(hello, engine) hello = withMemoryAppliesFeature(hello, engine) + hello = withDevicesFeatures(hello, engine) hello = withFactoryFeature(hello, engine) hello = withFactoryActorFeature(hello, engine) hello = withFactoryJobStepEditFeature(hello, engine) diff --git a/internal/chatlist/copy.go b/internal/chatlist/copy.go index 6b55a7ad10..2e70fa56de 100644 --- a/internal/chatlist/copy.go +++ b/internal/chatlist/copy.go @@ -19,6 +19,11 @@ const ( SyncOff = "sync is off: set CODEAF_SYNC_URL to your sync address" Unreachable = "other machines unreachable" ClockOff = "this computer's clock is off by more than 5 minutes" + + DevicesQuiet = "Only this computer is here. Add another computer by pairing from a terminal with `codeaf pair`." + RemoveSelf = "that is this device — remove it from another of your devices instead" + DeviceMissing = "that device is not in your list any more — `codeaf devices` shows the ones there are" + // What the relay's refusals say (one sentence each, in the order the // cellsync table lists them). None names a code or a number of requests: // each says what is true for the person and what, if anything, to do. diff --git a/internal/syncsetup/devices.go b/internal/syncsetup/devices.go new file mode 100644 index 0000000000..8d65815c56 --- /dev/null +++ b/internal/syncsetup/devices.go @@ -0,0 +1,280 @@ +package syncsetup + +import ( + "context" + "errors" + "maps" + "reflect" + "sync" + "time" + + "github.com/Agent-Field/codeaf/internal/devname" + "github.com/Agent-Field/codeaf/internal/directory" + "github.com/Agent-Field/codeaf/internal/dirwatch" + "github.com/Agent-Field/codeaf/internal/pair" +) + +// DeviceRequestWithin bounds a person's device request, including rename. +const DeviceRequestWithin = 20 * time.Second + +// DeviceSnapshot contains only facts already read by this process. A missing +// Online entry means unknown, including after a presence socket goes away. +type DeviceSnapshot struct { + Devices map[string]directory.Device + Online map[string]bool + Version uint64 + Refused error +} + +// DeviceObserver listens locally. It never opens a client, socket or timer. +type DeviceObserver struct { + home string + state DeviceSnapshot + changes chan struct{} +} + +var deviceObservers = struct { + sync.Mutex + all map[*DeviceObserver]struct{} +}{all: map[*DeviceObserver]struct{}{}} + +func ObserveDevices(home string) *DeviceObserver { + o := &DeviceObserver{home: home, changes: make(chan struct{}, 1)} + deviceObservers.Lock() + deviceObservers.all[o] = struct{}{} + deviceObservers.Unlock() + return o +} + +func (o *DeviceObserver) Changes() <-chan struct{} { return o.changes } + +func (o *DeviceObserver) Snapshot() DeviceSnapshot { + deviceObservers.Lock() + defer deviceObservers.Unlock() + s := o.state + s.Devices, s.Online = maps.Clone(s.Devices), maps.Clone(s.Online) + return s +} + +func (o *DeviceObserver) Close() { + deviceObservers.Lock() + defer deviceObservers.Unlock() + if _, ok := deviceObservers.all[o]; ok { + delete(deviceObservers.all, o) + close(o.changes) + } +} + +func rememberDevices(home string, edit func(*DeviceSnapshot)) { + deviceObservers.Lock() + defer deviceObservers.Unlock() + for o := range deviceObservers.all { + if o.home != home { + continue + } + next := o.state + next.Devices, next.Online = maps.Clone(next.Devices), maps.Clone(next.Online) + if next.Devices == nil { + next.Devices = map[string]directory.Device{} + } + edit(&next) + if reflect.DeepEqual(next, o.state) { + continue + } + o.state = next + select { + case o.changes <- struct{}{}: + default: + } + } +} + +// deviceDirectory keeps facts from requests the engine was making anyway. +// Embedding the HTTP client preserves its holding and conditional-take doors. +type deviceDirectory struct { + *directory.HTTP + home, self string +} + +func (d *deviceDirectory) List(ctx context.Context) (directory.Listing, error) { + l, err := d.HTTP.List(ctx) + d.refusal(err) + if err == nil { + rememberDevices(d.home, func(s *DeviceSnapshot) { + if l.Version != 0 && l.Version < s.Version { + return + } + s.Devices, s.Version = maps.Clone(l.Devices), l.Version + }) + } + return l, err +} + +func (d *deviceDirectory) PutDevice(ctx context.Context, id string, rec directory.Device) error { + err := d.HTTP.PutDevice(ctx, id, rec) + d.refusal(err) + if err == nil { + rememberDevices(d.home, func(s *DeviceSnapshot) { + old := s.Devices[id] + rec.Revoked, rec.Created, rec.LastSeen = old.Revoked, old.Created, old.LastSeen + s.Devices[id] = rec + }) + } + return err +} + +func (d *deviceDirectory) Revoke(ctx context.Context, id string) error { + err := d.HTTP.Revoke(ctx, id) + d.refusal(err) + if err == nil { + rememberDevices(d.home, func(s *DeviceSnapshot) { + rec := s.Devices[id] + rec.Revoked = true + s.Devices[id] = rec + delete(s.Online, id) + }) + } + return err +} + +func (d *deviceDirectory) Presence(ctx context.Context) (directory.PresenceView, error) { + p, err := d.HTTP.Presence(ctx) + d.refusal(err) + rememberDevices(d.home, func(s *DeviceSnapshot) { + s.Online = nil + if err != nil { + return + } + s.Online = map[string]bool{} + for id, seen := range p.Devices { + s.Online[id] = seen.Online + // An online answer stamps now, not a past sighting. Keeping + // that clock out prevents a refresh/event/refresh loop. + if rec, ok := s.Devices[id]; ok && !seen.Online { + rec.LastSeen = seen.LastSeen + s.Devices[id] = rec + } + } + }) + return p, err +} + +func (d *deviceDirectory) refusal(err error) { + var refused error + switch { + case errors.Is(err, directory.ErrRevoked): + refused = dirwatch.ErrRevoked + case errors.Is(err, directory.ErrRotated): + refused = dirwatch.ErrRotated + default: + return + } + rememberDevices(d.home, func(s *DeviceSnapshot) { + s.Refused, s.Online = refused, nil + if errors.Is(refused, dirwatch.ErrRevoked) { + rec := s.Devices[d.self] + rec.Revoked = true + s.Devices[d.self] = rec + } + }) +} + +func (d *deviceDirectory) Watch(ctx context.Context) (dirwatch.Stream, error) { + s, err := d.HTTP.Watch(ctx) + if err != nil { + d.refusal(err) + return nil, err + } + return &deviceStream{Stream: s, dir: d, presence: true}, nil +} + +func (d *deviceDirectory) WatchHolding(ctx context.Context, holds []directory.Hold) (dirwatch.Stream, error) { + s, err := d.HTTP.WatchHolding(ctx, holds) + if err != nil { + d.refusal(err) + return nil, err + } + return &deviceStream{Stream: s, dir: d}, nil +} + +type deviceStream struct { + dirwatch.Stream + dir *deviceDirectory + presence bool +} + +func (s *deviceStream) Next(ctx context.Context) (dirwatch.Frame, error) { + f, err := s.Stream.Next(ctx) + if f.Pong && err == nil { + return f, err + } + rememberDevices(s.dir.home, func(v *DeviceSnapshot) { + switch { + case err != nil: + delete(v.Online, s.dir.self) + if s.presence { + v.Online = nil + } + if errors.Is(err, dirwatch.ErrRevoked) || errors.Is(err, dirwatch.ErrRotated) { + v.Refused = err + if errors.Is(err, dirwatch.ErrRevoked) { + rec := v.Devices[s.dir.self] + rec.Revoked = true + v.Devices[s.dir.self] = rec + } + } + case f.Event != nil: + e := f.Event + rec := v.Devices[e.Device] + switch e.T { + case dirwatch.EventJoined: + rec.Name, rec.Platform = e.Name, e.Platform + v.Devices[e.Device] = rec + case dirwatch.EventRevoked: + rec.Revoked = true + v.Devices[e.Device] = rec + delete(v.Online, e.Device) + case dirwatch.EventPresence: + if e.Online != nil { + if v.Online == nil { + v.Online = map[string]bool{} + } + v.Online[e.Device] = *e.Online + if !*e.Online && e.At > 0 { + rec.LastSeen = e.At + v.Devices[e.Device] = rec + } + } + } + default: + v.Version = max(v.Version, f.Version) + if v.Online == nil { + v.Online = map[string]bool{} + } + v.Online[s.dir.self] = true + } + }) + return f, err +} + +// RenameDevice keeps the name here first, then tells the other computers when +// allowed. A failed relay write keeps the CLI's successful local-only receipt. +func RenameDevice(ctx context.Context, home, typed string) (string, error) { + name, err := devname.Set(home, typed) + if err != nil { + return "", err + } + if !MayTalk(home) { + return pair.RenamedAloneLine(name), nil + } + s, ok, err := Open(home) + if err != nil || !ok { + return pair.RenamedHereLine(name), nil + } + ctx, cancel := context.WithTimeout(ctx, DeviceRequestWithin) + defer cancel() + if err := s.Rename(ctx, name); err != nil { + return pair.RenamedHereLine(name), nil + } + return pair.RenamedLine(name), nil +} diff --git a/internal/syncsetup/syncsetup.go b/internal/syncsetup/syncsetup.go index 00fcac5f65..96b5a96cdf 100644 --- a/internal/syncsetup/syncsetup.go +++ b/internal/syncsetup/syncsetup.go @@ -164,7 +164,7 @@ func build(home, base string, id identity.Identity, dev identity.Dev, interval t sign := reqsign.SignFor(deviceSigner{id, dev}, time.Now) hc := gated(home, &http.Client{Timeout: requestTimeout}) return &Sync{ - Dir: directory.NewHTTP(base, sign, hc), + Dir: &deviceDirectory{HTTP: directory.NewHTTP(base, sign, hc), home: home, self: dev.ID()}, Store: blobstore.NewHTTP(base, sign, gated(home, &http.Client{})), Device: dev, Identity: id, From 290a8490f3ff007bb98870d52bf5bad48a83ba30 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 15:44:37 -0400 Subject: [PATCH 2/7] fix(app): supply engine removal copy before confirmation Add optional Device.remove_line to the app roster and generated schema. Supply the existing target-specific engine removal receipt only on active other computers, so the desktop can show the engine's explanation before confirmation. Keep the devices.remove result unchanged and omit copy on self and removed rows. Extend real paired-home appwire coverage for list/event copy and omission, and check the exact JSON and optional schema field. Update the manual and change entry to document the additive field and replace the old copy assumption. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/codeaf/appdevices.go | 5 +- cmd/codeaf/appdevices_test.go | 122 +++++++++++++++++- docs/changes/unreleased/0-app-devices.md | 13 ++ internal/appwire/devices.go | 16 +-- internal/appwire/devices_test.go | 22 ++++ .../manual/chat/add-or-remove-a-computer.md | 18 +++ 6 files changed, 184 insertions(+), 12 deletions(-) diff --git a/cmd/codeaf/appdevices.go b/cmd/codeaf/appdevices.go index 94488edf5f..e2c7ee630c 100644 --- a/cmd/codeaf/appdevices.go +++ b/cmd/codeaf/appdevices.go @@ -63,7 +63,7 @@ func (d *appDevices) snapshot() (appwire.Devices, error) { return appwire.Devices{}, err } view := appwire.Devices{Mode: appwire.DevicesOn} - known := d.watch.Snapshot() + known := d.watch.Snapshot(id, self) switch { case !syncsetup.Resolve(d.home).On(): view.Mode, view.Line = appwire.DevicesOff, chatlist.SyncOff @@ -90,6 +90,9 @@ func (d *appDevices) snapshot() (appwire.Devices, error) { view.Devices = make([]appwire.Device, 0, len(known.Devices)+1) for deviceID, rec := range known.Devices { row := appwire.Device{ID: deviceID, Name: chatlist.DeviceName(known.Devices, deviceID, open), Platform: rec.Platform, Self: deviceID == self.ID(), Removed: rec.Revoked} + if view.Mode == appwire.DevicesOn && !row.Self && !row.Removed { + row.RemoveLine = pair.ChatsRevokedLine(row.Name) + } if rec.LastSeen > 0 { row.LastSeen = time.UnixMilli(rec.LastSeen).UTC() } diff --git a/cmd/codeaf/appdevices_test.go b/cmd/codeaf/appdevices_test.go index 947dd7371f..084d2413f9 100644 --- a/cmd/codeaf/appdevices_test.go +++ b/cmd/codeaf/appdevices_test.go @@ -97,6 +97,115 @@ func appDeviceEvent(t *testing.T, rec *appRecorder, match func(appwire.Devices) }) } +// Hold one completed signed list response at the HTTP boundary. Other requests +// still reach the real directory, including the list and revoke of a removal. +type appDeviceDelayedList struct { + armed atomic.Bool + ready chan struct{} + release chan struct{} + once sync.Once +} + +func (d *appDeviceDelayedList) wrap(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/dir/list" || !d.armed.CompareAndSwap(true, false) { + next.ServeHTTP(w, r) + return + } + reply := httptest.NewRecorder() + next.ServeHTTP(reply, r) + close(d.ready) + select { + case <-d.release: + case <-r.Context().Done(): + return + } + for key, values := range reply.Header() { + w.Header()[key] = slices.Clone(values) + } + w.WriteHeader(reply.Code) + _, _ = w.Write(reply.Body.Bytes()) + }) +} + +func (d *appDeviceDelayedList) resume() { d.once.Do(func() { close(d.release) }) } + +func (d *appDeviceDelayedList) wait(t *testing.T) { + t.Helper() + select { + case <-d.ready: + case <-time.After(5 * time.Second): + t.Fatal("the signed roster response was not captured") + } +} + +// V1, V3 amendment: a completed removal survives an older concurrent roster +// response from another sync client, even when the next refresh fails. +func TestAppDevicesV1V3OlderRosterCannotUndoRemoval(t *testing.T) { + celltest.Available(t) + delay := &appDeviceDelayedList{ready: make(chan struct{}), release: make(chan struct{})} + _, _, down := appDevicesRelay(t, delay.wrap) + t.Cleanup(delay.resume) + a, b := t.TempDir(), t.TempDir() + id, err := identity.Ensure(a) + if err != nil { + t.Fatal(err) + } + if _, err := identity.Adopt(b, id, false); err != nil { + t.Fatal(err) + } + sa, sb := appDeviceSync(t, a), appDeviceSync(t, b) + for _, s := range []*syncsetup.Sync{sa, sb} { + if err := s.Rename(context.Background(), "computer"); err != nil { + t.Fatal(err) + } + } + _, rec, w := appDeviceBackend(t, a) + if view := appDeviceList(t, w); len(view.Devices) != 2 || view.Devices[1].Removed { + t.Fatalf("initial roster: %+v", view) + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + type response struct { + listing directory.Listing + err error + } + done := make(chan response, 1) + delay.armed.Store(true) + go func() { + listing, err := sa.Dir.List(ctx) + done <- response{listing, err} + }() + delay.wait(t) + removed := machineCall[appwire.DeviceRemoveResult](t, w, "devices.remove", appwire.DeviceRemoveParams{ID: sb.Device.ID()}) + if removed.ID != sb.Device.ID() || removed.Line != pair.ChatsRevokedLine("computer") { + t.Fatalf("removal receipt: %+v", removed) + } + appDeviceEvent(t, rec, func(v appwire.Devices) bool { return len(v.Devices) == 2 && v.Devices[1].Removed }) + down.Store(true) + delay.resume() + select { + case stale := <-done: + if stale.err != nil || stale.listing.Version == 0 || stale.listing.Devices[sb.Device.ID()].Revoked { + t.Fatalf("the delayed response was not the older active roster: %+v, %v", stale.listing, stale.err) + } + case <-ctx.Done(): + t.Fatal("the delayed roster response did not finish") + } + if _, err := w.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeUnreachable { + t.Fatalf("follow-up refresh must fail: %v", err) + } + // An offline rename publishes the retained roster through the real app + // event door, without a successful relay read to repair a resurrection. + renamed := machineCall[appwire.DeviceRenameResult](t, w, "devices.rename", appwire.DeviceRenameParams{Name: "offline desk"}) + if renamed.Line != pair.RenamedHereLine("offline desk") { + t.Fatalf("offline rename receipt: %+v", renamed) + } + appDeviceEvent(t, rec, func(v appwire.Devices) bool { + return len(v.Devices) == 2 && v.Devices[0].Name == "offline desk" && v.Devices[1].Removed && v.Devices[1].RemoveLine == "" && v.Devices[1].Online == nil + }) +} + // V1, V5: real watch traffic updates presence without any roster/presence read. func TestAppDevicesV1V5ExistingWatchSuppliesPresence(t *testing.T) { celltest.Available(t) @@ -221,6 +330,11 @@ func TestAppDevicesV1V2V3PairedHomes(t *testing.T) { if view.Mode != appwire.DevicesOn || len(view.Devices) != 2 || view.Devices[0].ID != sa.Device.ID() || !view.Devices[0].Self || view.Devices[0].Name != "desk" || view.Devices[1].Name != "laptop" || view.Devices[1].Self || view.Devices[1].Platform != runtime.GOOS || view.Devices[1].Removed { t.Fatalf("paired list: %+v", view) } + // V3 amendment: the engine's target-specific explanation is available + // before removal, and only on a row this computer may remove. + if view.Devices[0].RemoveLine != "" || view.Devices[1].RemoveLine != pair.ChatsRevokedLine("laptop") { + t.Fatalf("removal confirmation copy: %+v", view.Devices) + } for _, params := range []map[string]any{{"name": ""}, {"name": strings.Repeat("x", devname.Most+1)}, {"name": "a\nb"}, {"name": "other", "id": sb.Device.ID()}} { if _, err := wa.call("devices.rename", params); err == nil || err.Code != appwire.CodeBadParams { t.Fatalf("rename %v: %v", params, err) @@ -230,7 +344,9 @@ func TestAppDevicesV1V2V3PairedHomes(t *testing.T) { if renamed.Device.ID != sa.Device.ID() || renamed.Device.Name != "studio" || renamed.Line != pair.RenamedLine("studio") { t.Fatalf("rename receipt: %+v", renamed) } - appDeviceEvent(t, rec, func(v appwire.Devices) bool { return v.Devices[0].Name == "studio" }) + appDeviceEvent(t, rec, func(v appwire.Devices) bool { + return len(v.Devices) == 2 && v.Devices[0].Name == "studio" && v.Devices[0].RemoveLine == "" && v.Devices[1].RemoveLine == pair.ChatsRevokedLine("laptop") + }) other := appDeviceList(t, wb) if len(other.Devices) != 2 || other.Devices[0].ID != sb.Device.ID() || other.Devices[1].Name != "studio" { t.Fatalf("the other home sees: %+v", other) @@ -256,7 +372,7 @@ func TestAppDevicesV1V2V3PairedHomes(t *testing.T) { } appDeviceEvent(t, rec, func(v appwire.Devices) bool { return len(v.Devices) == 2 && v.Devices[1].Removed }) view = appDeviceList(t, wa) - if len(view.Devices) != 2 || !view.Devices[1].Removed { + if len(view.Devices) != 2 || !view.Devices[1].Removed || view.Devices[1].RemoveLine != "" { t.Fatalf("removed device disappeared: %+v", view) } if _, err := wb.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeEngine || err.Message != chatlist.Removed { @@ -302,7 +418,7 @@ func TestAppDevicesV4V5LocalModesMakeNoRelayRequests(t *testing.T) { } backend, rec, w := appDeviceBackend(t, dir) view := appDeviceList(t, w) - if view.Mode != mode || len(view.Devices) != 1 || !view.Devices[0].Self || view.Devices[0].Online != nil || !view.Devices[0].LastSeen.IsZero() || view.Line == "" { + if view.Mode != mode || len(view.Devices) != 1 || !view.Devices[0].Self || view.Devices[0].Online != nil || !view.Devices[0].LastSeen.IsZero() || view.Devices[0].RemoveLine != "" || view.Line == "" { t.Fatalf("local list: %+v", view) } if mode == appwire.DevicesQuiet && (view.Line != chatlist.DevicesQuiet || !strings.Contains(view.Line, "codeaf pair") || !identity.Solo(dir)) { diff --git a/docs/changes/unreleased/0-app-devices.md b/docs/changes/unreleased/0-app-devices.md index 2e7ca2674f..426d3d73d1 100644 --- a/docs/changes/unreleased/0-app-devices.md +++ b/docs/changes/unreleased/0-app-devices.md @@ -5,6 +5,9 @@ pr: 0 surface: [engine] invalidates: - "The app engine had no device-management methods. Engines that advertise devices.manage now serve devices.list, devices.rename and devices.remove." + - "The app roster previously supplied removal copy only after removal. Removable rows now carry the engine's target-specific remove_line for confirmation." + - "An older concurrent roster read could restore a removed computer's active state. Completed removals now survive older responses and failed refreshes." + - "The Devices observer previously kept the old pairing's roster and refusal until restart. It now resets when the relay, identity or device changes and ignores old clients' callbacks." --- The app wire lists this computer first and includes other computers paired to @@ -12,6 +15,8 @@ the same identity, their platform, removal state, last sighting and known presence. Unknown presence is omitted. Only this computer can be renamed; another computer can be removed by its id. Receipts use the engine's existing rename and removal sentences, including the limits of removing a lost computer. +Removable rows also include the optional `remove_line`, using that same +target-specific engine copy before confirmation; self and removed rows omit it. Never-paired homes list only this computer and explain how to add another with `codeaf pair` from a terminal. Sync-off and unavailable homes also answer locally. @@ -20,3 +25,11 @@ socket: an explicit list reads the roster and presence, while `devices.changed` reuses facts received by existing engine requests and sockets. Rename and remove emit a snapshot without a follow-up relay read, preserving a completed change when a later refresh fails. Pairing itself and the desktop Settings UI are separate. + +An older concurrent roster response cannot undo a completed removal. Equal or +unversioned roster reads retain removed records until a newer directory version +says otherwise. + +After terminal re-pairing, the running app engine drops the old +roster, version, presence and refusal on its next snapshot. Late responses and +watch frames from the former relay, identity or device cannot restore those facts. diff --git a/internal/appwire/devices.go b/internal/appwire/devices.go index b2651207dd..878cbe1df0 100644 --- a/internal/appwire/devices.go +++ b/internal/appwire/devices.go @@ -1,7 +1,6 @@ package appwire import ( - "encoding/json" "time" ) @@ -16,13 +15,14 @@ const ( // Device is a computer paired to this identity. Unknown presence is absent. type Device struct { - ID string `json:"id"` - Name string `json:"name"` - Platform string `json:"platform,omitempty"` - Self bool `json:"self"` - Removed bool `json:"removed"` - LastSeen time.Time `json:"last_seen,omitzero"` - Online *bool `json:"online,omitempty"` + ID string `json:"id"` + Name string `json:"name"` + Platform string `json:"platform,omitempty"` + Self bool `json:"self"` + Removed bool `json:"removed"` + LastSeen time.Time `json:"last_seen,omitzero"` + Online *bool `json:"online,omitempty"` + RemoveLine string `json:"remove_line,omitempty"` } type Devices struct { diff --git a/internal/appwire/devices_test.go b/internal/appwire/devices_test.go index 5a2aefcc30..a7830f3976 100644 --- a/internal/appwire/devices_test.go +++ b/internal/appwire/devices_test.go @@ -70,3 +70,25 @@ func TestDevicesV1V5WireShapes(t *testing.T) { } } } + +// V3 and the amendment: removal copy is optional on both roster envelopes. +func TestDevicesV3RemovalExplanationSchemaAndWire(t *testing.T) { + line := "laptop stops syncing; its saved chats and keys stay there." + device := Device{ID: "other", Name: "laptop", RemoveLine: line} + raw, err := json.Marshal(DevicesResult{Devices: Devices{Devices: []Device{device}, Mode: DevicesOn}}) + want := `{"devices":{"devices":[{"id":"other","name":"laptop","self":false,"removed":false,"remove_line":"laptop stops syncing; its saved chats and keys stay there."}],"mode":"on"}}` + if err != nil || string(raw) != want { + t.Fatalf("removal explanation = %s, %v; want %s", raw, err, want) + } + if EventShape(EventDevicesChanged) != reflect.TypeFor[DevicesResult]() { + t.Fatal("devices.changed lost the roster's removal explanation") + } + def := Schema()["$defs"].(map[string]any)["Device"].(map[string]any) + props := def["properties"].(map[string]any) + if !reflect.DeepEqual(props["remove_line"], map[string]any{"type": "string"}) { + t.Fatalf("remove_line schema = %#v", props["remove_line"]) + } + if slices.Contains(def["required"].([]string), "remove_line") { + t.Fatal("remove_line must be optional for self and removed rows") + } +} diff --git a/internal/manual/chat/add-or-remove-a-computer.md b/internal/manual/chat/add-or-remove-a-computer.md index a1da3031dd..f1a8904c09 100644 --- a/internal/manual/chat/add-or-remove-a-computer.md +++ b/internal/manual/chat/add-or-remove-a-computer.md @@ -164,6 +164,24 @@ desk was removed — it can no longer reach your chats. The list is in a fixed order: this device first, then devices that are online, then the rest, each by name. Two devices with the same name show a short tail, such as `tower #a1b2`, so you can tell them apart. Your own computer has no `r`: it cannot remove itself, and if you press `r` there the list says `this device cannot remove itself — choose another row.` A computer that is already removed does nothing when you press `r`. A removed computer can only come back as a new request that you approve. If you remove a computer from another of your devices while this list is open, its row changes to `removed` here too, without closing the list. Removing a computer does not undo what it already holds: for a lost computer read the page *Locking out a lost computer for good*. +## Devices in the desktop — removal warnings, refresh and re-pairing + +With an engine that supports device management, the desktop can list this +computer first and the other computers paired to the same identity. The engine +supplies its removal explanation for each active computer that this one may +remove, so it can be shown before confirmation. This computer and already removed +computers have no removal action. The result keeps the command line's full +explanation, including the warning about chats and keys already held there. + +After a removal succeeds, an older pending roster response cannot restore the +computer's active state. If a refresh fails, the completed removal is kept; only +a roster at a newer directory version can change that state. + +After pairing again from a terminal, refresh Devices. The running app engine +reads the new identity and device files and drops the previous pairing's list, +presence and removal refusal. Restarting the engine is not required. Switching +the sync address also drops facts learned from the previous service. + ## I was told this device was removed — how to bring it back Moving chats and pairing are available by default on Linux, macOS and native Windows on every build with a compatible engine. Pair both computers first. From 5ca8cce1cfad2f8870f0264950777cbce3c2821c Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 15:44:37 -0400 Subject: [PATCH 3/7] fix(sync): retain device removals across stale roster reads Order roster reads against completed removals across sync clients, including clients outside the app device-operation mutex. Reject responses begun before the latest removal and retain removed records on equal-version or unversioned reads until a newer directory version says otherwise. Add a regression using the real appwire backend and signed in-process directory: delay an active roster response, remove through the app, release the older response, fail refresh and require the retained removed record in an app event. Document the corrected retention behavior and obsolete ordering assumption. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/syncsetup/devices.go | 96 +++++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 22 deletions(-) diff --git a/internal/syncsetup/devices.go b/internal/syncsetup/devices.go index 8d65815c56..33fca595ab 100644 --- a/internal/syncsetup/devices.go +++ b/internal/syncsetup/devices.go @@ -6,11 +6,13 @@ import ( "maps" "reflect" "sync" + "sync/atomic" "time" "github.com/Agent-Field/codeaf/internal/devname" "github.com/Agent-Field/codeaf/internal/directory" "github.com/Agent-Field/codeaf/internal/dirwatch" + "github.com/Agent-Field/codeaf/internal/identity" "github.com/Agent-Field/codeaf/internal/pair" ) @@ -20,19 +22,36 @@ const DeviceRequestWithin = 20 * time.Second // DeviceSnapshot contains only facts already read by this process. A missing // Online entry means unknown, including after a presence socket goes away. type DeviceSnapshot struct { - Devices map[string]directory.Device - Online map[string]bool - Version uint64 - Refused error + Devices map[string]directory.Device + Online map[string]bool + Version uint64 + Refused error + mutation uint64 } +// A removal's sequence orders it against reads from every client in this +// process, including sync work outside the app's device-operation mutex. +var deviceMutations atomic.Uint64 + // DeviceObserver listens locally. It never opens a client, socket or timer. type DeviceObserver struct { home string + binding deviceBinding state DeviceSnapshot changes chan struct{} } +type deviceBinding struct { + relay, identity, self string +} + +func deviceBindingFor(relay string, id identity.Identity, dev identity.Dev) deviceBinding { + if base, err := relayBase(relay); err == nil { + relay = base + } + return deviceBinding{relay: relay, identity: id.ID(), self: dev.ID()} +} + var deviceObservers = struct { sync.Mutex all map[*DeviceObserver]struct{} @@ -40,6 +59,11 @@ var deviceObservers = struct { func ObserveDevices(home string) *DeviceObserver { o := &DeviceObserver{home: home, changes: make(chan struct{}, 1)} + if id, err := identity.Load(home); err == nil { + if dev, err := identity.Device(home); err == nil { + o.binding = deviceBindingFor(Resolve(home).URL, id, dev) + } + } deviceObservers.Lock() deviceObservers.all[o] = struct{}{} deviceObservers.Unlock() @@ -48,9 +72,15 @@ func ObserveDevices(home string) *DeviceObserver { func (o *DeviceObserver) Changes() <-chan struct{} { return o.changes } -func (o *DeviceObserver) Snapshot() DeviceSnapshot { +// Snapshot scopes remembered facts to the pairing files the caller just read. +// Clients of a previous pairing cannot restore its roster or refusal later. +func (o *DeviceObserver) Snapshot(id identity.Identity, dev identity.Dev) DeviceSnapshot { + binding := deviceBindingFor(Resolve(o.home).URL, id, dev) deviceObservers.Lock() defer deviceObservers.Unlock() + if o.binding != binding { + o.binding, o.state = binding, DeviceSnapshot{} + } s := o.state s.Devices, s.Online = maps.Clone(s.Devices), maps.Clone(s.Online) return s @@ -65,11 +95,11 @@ func (o *DeviceObserver) Close() { } } -func rememberDevices(home string, edit func(*DeviceSnapshot)) { +func rememberDevices(home string, binding deviceBinding, edit func(*DeviceSnapshot)) { deviceObservers.Lock() defer deviceObservers.Unlock() for o := range deviceObservers.all { - if o.home != home { + if o.home != home || o.binding != binding { continue } next := o.state @@ -93,18 +123,38 @@ func rememberDevices(home string, edit func(*DeviceSnapshot)) { // Embedding the HTTP client preserves its holding and conditional-take doors. type deviceDirectory struct { *directory.HTTP - home, self string + home string + binding deviceBinding } func (d *deviceDirectory) List(ctx context.Context) (directory.Listing, error) { + read := deviceMutations.Load() l, err := d.HTTP.List(ctx) d.refusal(err) if err == nil { - rememberDevices(d.home, func(s *DeviceSnapshot) { - if l.Version != 0 && l.Version < s.Version { + rememberDevices(d.home, d.binding, func(s *DeviceSnapshot) { + if read < s.mutation || l.Version != 0 && l.Version < s.Version { return } - s.Devices, s.Version = maps.Clone(l.Devices), l.Version + devices := maps.Clone(l.Devices) + if devices == nil { + devices = map[string]directory.Device{} + } + // Equal or unversioned reads cannot retract a removal. A read + // begun before a completed removal cannot replace any facts. + if l.Version == 0 || l.Version == s.Version { + for id, old := range s.Devices { + if old.Revoked { + rec, ok := devices[id] + if !ok { + rec = old + } + rec.Revoked = true + devices[id] = rec + } + } + } + s.Devices, s.Version = devices, max(s.Version, l.Version) }) } return l, err @@ -114,7 +164,7 @@ func (d *deviceDirectory) PutDevice(ctx context.Context, id string, rec director err := d.HTTP.PutDevice(ctx, id, rec) d.refusal(err) if err == nil { - rememberDevices(d.home, func(s *DeviceSnapshot) { + rememberDevices(d.home, d.binding, func(s *DeviceSnapshot) { old := s.Devices[id] rec.Revoked, rec.Created, rec.LastSeen = old.Revoked, old.Created, old.LastSeen s.Devices[id] = rec @@ -127,7 +177,9 @@ func (d *deviceDirectory) Revoke(ctx context.Context, id string) error { err := d.HTTP.Revoke(ctx, id) d.refusal(err) if err == nil { - rememberDevices(d.home, func(s *DeviceSnapshot) { + mutation := deviceMutations.Add(1) + rememberDevices(d.home, d.binding, func(s *DeviceSnapshot) { + s.mutation = max(s.mutation, mutation) rec := s.Devices[id] rec.Revoked = true s.Devices[id] = rec @@ -140,7 +192,7 @@ func (d *deviceDirectory) Revoke(ctx context.Context, id string) error { func (d *deviceDirectory) Presence(ctx context.Context) (directory.PresenceView, error) { p, err := d.HTTP.Presence(ctx) d.refusal(err) - rememberDevices(d.home, func(s *DeviceSnapshot) { + rememberDevices(d.home, d.binding, func(s *DeviceSnapshot) { s.Online = nil if err != nil { return @@ -169,12 +221,12 @@ func (d *deviceDirectory) refusal(err error) { default: return } - rememberDevices(d.home, func(s *DeviceSnapshot) { + rememberDevices(d.home, d.binding, func(s *DeviceSnapshot) { s.Refused, s.Online = refused, nil if errors.Is(refused, dirwatch.ErrRevoked) { - rec := s.Devices[d.self] + rec := s.Devices[d.binding.self] rec.Revoked = true - s.Devices[d.self] = rec + s.Devices[d.binding.self] = rec } }) } @@ -208,19 +260,19 @@ func (s *deviceStream) Next(ctx context.Context) (dirwatch.Frame, error) { if f.Pong && err == nil { return f, err } - rememberDevices(s.dir.home, func(v *DeviceSnapshot) { + rememberDevices(s.dir.home, s.dir.binding, func(v *DeviceSnapshot) { switch { case err != nil: - delete(v.Online, s.dir.self) + delete(v.Online, s.dir.binding.self) if s.presence { v.Online = nil } if errors.Is(err, dirwatch.ErrRevoked) || errors.Is(err, dirwatch.ErrRotated) { v.Refused = err if errors.Is(err, dirwatch.ErrRevoked) { - rec := v.Devices[s.dir.self] + rec := v.Devices[s.dir.binding.self] rec.Revoked = true - v.Devices[s.dir.self] = rec + v.Devices[s.dir.binding.self] = rec } } case f.Event != nil: @@ -251,7 +303,7 @@ func (s *deviceStream) Next(ctx context.Context) (dirwatch.Frame, error) { if v.Online == nil { v.Online = map[string]bool{} } - v.Online[s.dir.self] = true + v.Online[s.dir.binding.self] = true } }) return f, err From 7747262393eebbafea7e0cba6c7a5cc9d633e24f Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 15:44:37 -0400 Subject: [PATCH 4/7] fix(sync): reset device observations after re-pairing Scope device observations to the normalized relay address, identity and device. Reset roster, version, presence, refusal and removal ordering when the app reads a different local binding. Reject later HTTP and watch updates from old clients. Cover successful terminal re-pairing after learned self removal on the same app backend, replacement by an identity with a lower directory version, late old responses/watch facts and a relay change with unchanged identity/device files. Use real homes, registered appwire, signed HTTP and in-process relays. Update the manual and change entry to explain recovery without restarting the engine. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/codeaf/appdevices_binding_test.go | 235 ++++++++++++++++++++++++++ internal/syncsetup/syncsetup.go | 2 +- 2 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 cmd/codeaf/appdevices_binding_test.go diff --git a/cmd/codeaf/appdevices_binding_test.go b/cmd/codeaf/appdevices_binding_test.go new file mode 100644 index 0000000000..7d024c5f39 --- /dev/null +++ b/cmd/codeaf/appdevices_binding_test.go @@ -0,0 +1,235 @@ +package main + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/cell/celltest" + "github.com/Agent-Field/codeaf/internal/chatlist" + "github.com/Agent-Field/codeaf/internal/devname" + "github.com/Agent-Field/codeaf/internal/directory" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/identity" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +// Use the same terminal doors as codeaf pair and codeaf pair approve, with +// real link approval, identity/device installation and own-device publication. +func appDevicePairByLink(t *testing.T, rig *pairRig, args ...string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + screen, done := &pairScreenText{}, make(chan error, 1) + go func() { done <- rig.door(rig.homeB, strings.NewReader(""), screen).run(ctx, args) }() + typed := screen.waitFor(t, `codeaf pair approve (\S+)`)[1] + if err := rig.door(rig.homeA, strings.NewReader("a\n"), &pairScreenText{}).run(ctx, []string{"approve", typed}); err != nil { + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-ctx.Done(): + t.Fatal("terminal pairing did not finish") + } +} + +// V1, V3 amendment: terminal re-pairing clears a learned self removal on the +// same running app backend, even when an old client later reports its refusal. +func TestAppDevicesV1V3RePairClearsRemovalWithoutRestart(t *testing.T) { + rig := newPairRig(t) + t.Setenv(home.EnvVar, rig.homeA) + t.Setenv(syncsetup.URLVar, rig.url) + t.Setenv(syncsetup.IntervalVar, "") + appDevicePairByLink(t, rig) + sa, old := appDeviceSync(t, rig.homeA), appDeviceSync(t, rig.homeB) + _, _, w := appDeviceBackend(t, rig.homeB) + if view := appDeviceList(t, w); len(view.Devices) != 2 || view.Devices[0].ID != old.Device.ID() { + t.Fatalf("initial pairing: %+v", view) + } + if err := sa.Dir.Revoke(context.Background(), old.Device.ID()); err != nil { + t.Fatal(err) + } + if _, err := w.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeEngine || err.Message != chatlist.Removed { + t.Fatalf("learned removal: %v", err) + } + if view := appDeviceList(t, w); view.Mode != appwire.DevicesUnavailable || view.Line != chatlist.Removed || len(view.Devices) != 1 || !view.Devices[0].Removed { + t.Fatalf("remembered self removal: %+v", view) + } + appDevicePairByLink(t, rig, "--replace") + fresh := appDeviceSync(t, rig.homeB) + if fresh.Identity.ID() != old.Identity.ID() || fresh.Device.ID() == old.Device.ID() { + t.Fatal("terminal re-pairing must keep the identity and install a new device") + } + for _, late := range []bool{false, true} { + if late { + if _, err := old.Dir.List(context.Background()); !errors.Is(err, directory.ErrRevoked) { + t.Fatalf("the old client must still be refused: %v", err) + } + } + view := appDeviceList(t, w) + if view.Mode != appwire.DevicesOn || view.Line != "" || len(view.Devices) != 3 || view.Devices[0].ID != fresh.Device.ID() || !view.Devices[0].Self || view.Devices[0].Removed || view.Devices[0].RemoveLine != "" { + t.Fatalf("re-paired roster (late refusal %v): %+v", late, view) + } + at := slices.IndexFunc(view.Devices, func(row appwire.Device) bool { return row.ID == old.Device.ID() }) + if at < 0 || !view.Devices[at].Removed || view.Devices[at].Self || view.Devices[at].RemoveLine != "" { + t.Fatalf("the old device must remain a separate removed row: %+v", view) + } + } +} + +// V1 amendment: replacing the pairing accepts its lower directory version and +// drops the old roster, presence and callbacks, including a delayed old list. +func TestAppDevicesV1ReplacementPairingDropsOldRoster(t *testing.T) { + celltest.Available(t) + delay := &appDeviceDelayedList{ready: make(chan struct{}), release: make(chan struct{})} + _, _, down := appDevicesRelay(t, delay.wrap) + t.Cleanup(delay.resume) + a, b, c := t.TempDir(), t.TempDir(), t.TempDir() + id, err := identity.Ensure(a) + if err != nil { + t.Fatal(err) + } + if _, err := identity.Adopt(b, id, false); err != nil { + t.Fatal(err) + } + old, peer := appDeviceSync(t, a), appDeviceSync(t, b) + for i := 0; i < 8; i++ { + if err := old.Rename(context.Background(), "old desk "+strings.Repeat("x", i)); err != nil { + t.Fatal(err) + } + } + if err := peer.Rename(context.Background(), "old partner"); err != nil { + t.Fatal(err) + } + _, rec, w := appDeviceBackend(t, a) + _ = appDeviceList(t, w) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + before, err := old.Dir.List(ctx) + if err != nil { + t.Fatal(err) + } + watch, err := old.Dir.(directory.Watcher).Watch(ctx) + if err != nil { + t.Fatal(err) + } + defer watch.Close() + done := make(chan error, 1) + delay.armed.Store(true) + go func() { _, err := old.Dir.List(ctx); done <- err }() + delay.wait(t) + nextID, err := identity.Ensure(c) + if err != nil { + t.Fatal(err) + } + if _, err := identity.Adopt(a, nextID, true); err != nil { + t.Fatal(err) + } + fresh, partner := appDeviceSync(t, a), appDeviceSync(t, c) + for _, s := range []*syncsetup.Sync{fresh, partner} { + if _, err := devname.Set(s.Home, "new computer"); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "new computer"); err != nil { + t.Fatal(err) + } + } + after, err := partner.Dir.List(ctx) + if err != nil || after.Version == 0 || after.Version >= before.Version { + t.Fatalf("replacement directory must have a lower version: %d -> %d, %v", before.Version, after.Version, err) + } + view := appDeviceList(t, w) + if view.Mode != appwire.DevicesOn || len(view.Devices) != 2 || view.Devices[0].ID != fresh.Device.ID() || view.Devices[1].ID != partner.Device.ID() { + t.Fatalf("replacement pairing retained old ids or version: %+v", view) + } + delay.resume() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-ctx.Done(): + t.Fatal("the old pairing's delayed list did not finish") + } + // The old socket and clients remain alive; their later facts must not + // enter the observer now bound to the replacement pairing. + if _, err := watch.Next(ctx); err != nil { + t.Fatal(err) + } + if err := old.Rename(ctx, "stale old desk"); err != nil { + t.Fatal(err) + } + if _, err := old.Dir.Presence(ctx); err != nil { + t.Fatal(err) + } + if err := old.Dir.Revoke(ctx, peer.Device.ID()); err != nil { + t.Fatal(err) + } + down.Store(true) + _ = machineCall[appwire.DeviceRenameResult](t, w, "devices.rename", appwire.DeviceRenameParams{Name: "new cached desk"}) + appDeviceEvent(t, rec, func(v appwire.Devices) bool { + return v.Mode == appwire.DevicesOn && len(v.Devices) == 2 && v.Devices[0].ID == fresh.Device.ID() && v.Devices[0].Name == "new cached desk" && v.Devices[1].ID == partner.Device.ID() && !v.Devices[0].Removed && !v.Devices[1].Removed && v.Devices[0].Online != nil && !*v.Devices[0].Online && v.Devices[1].Online != nil && !*v.Devices[1].Online + }) +} + +// V1 amendment: changing the relay with unchanged identity/device files also +// drops the former relay's roster, version and later callbacks. +func TestAppDevicesV1RelayChangeDropsOldRoster(t *testing.T) { + celltest.Available(t) + _, _, _ = appDevicesRelay(t) + a, b, c := t.TempDir(), t.TempDir(), t.TempDir() + id, err := identity.Ensure(a) + if err != nil { + t.Fatal(err) + } + for _, dir := range []string{b, c} { + if _, err := identity.Adopt(dir, id, false); err != nil { + t.Fatal(err) + } + } + old, peer := appDeviceSync(t, a), appDeviceSync(t, b) + for i := 0; i < 5; i++ { + if err := old.Rename(context.Background(), "old desk"); err != nil { + t.Fatal(err) + } + } + if err := peer.Rename(context.Background(), "old partner"); err != nil { + t.Fatal(err) + } + _, rec, w := appDeviceBackend(t, a) + if view := appDeviceList(t, w); len(view.Devices) != 2 || view.Devices[1].ID != peer.Device.ID() { + t.Fatalf("former relay's roster: %+v", view) + } + _, _, down := appDevicesRelay(t) + fresh, partner := appDeviceSync(t, a), appDeviceSync(t, c) + if fresh.Identity.ID() != old.Identity.ID() || fresh.Device.ID() != old.Device.ID() { + t.Fatal("only the relay should change") + } + for _, s := range []*syncsetup.Sync{fresh, partner} { + if err := s.Rename(context.Background(), "new computer"); err != nil { + t.Fatal(err) + } + } + view := appDeviceList(t, w) + if view.Mode != appwire.DevicesOn || len(view.Devices) != 2 || view.Devices[0].ID != fresh.Device.ID() || view.Devices[1].ID != partner.Device.ID() { + t.Fatalf("new relay retained the former roster or version: %+v", view) + } + if _, err := old.Dir.List(context.Background()); err != nil { + t.Fatal(err) + } + if err := old.Dir.Revoke(context.Background(), peer.Device.ID()); err != nil { + t.Fatal(err) + } + down.Store(true) + _ = machineCall[appwire.DeviceRenameResult](t, w, "devices.rename", appwire.DeviceRenameParams{Name: "new relay desk"}) + appDeviceEvent(t, rec, func(v appwire.Devices) bool { + return len(v.Devices) == 2 && v.Devices[0].Name == "new relay desk" && v.Devices[1].ID == partner.Device.ID() && !v.Devices[1].Removed + }) +} diff --git a/internal/syncsetup/syncsetup.go b/internal/syncsetup/syncsetup.go index 96b5a96cdf..05d0934e57 100644 --- a/internal/syncsetup/syncsetup.go +++ b/internal/syncsetup/syncsetup.go @@ -164,7 +164,7 @@ func build(home, base string, id identity.Identity, dev identity.Dev, interval t sign := reqsign.SignFor(deviceSigner{id, dev}, time.Now) hc := gated(home, &http.Client{Timeout: requestTimeout}) return &Sync{ - Dir: &deviceDirectory{HTTP: directory.NewHTTP(base, sign, hc), home: home, self: dev.ID()}, + Dir: &deviceDirectory{HTTP: directory.NewHTTP(base, sign, hc), home: home, binding: deviceBindingFor(base, id, dev)}, Store: blobstore.NewHTTP(base, sign, gated(home, &http.Client{})), Device: dev, Identity: id, From 846e7e63ef7216bc35e37167bfa9ed36c512a15f Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 15:54:29 -0400 Subject: [PATCH 5/7] docs: attach the devices change entry to PR 1977 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../unreleased/{0-app-devices.md => 1977-app-devices.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/changes/unreleased/{0-app-devices.md => 1977-app-devices.md} (99%) diff --git a/docs/changes/unreleased/0-app-devices.md b/docs/changes/unreleased/1977-app-devices.md similarity index 99% rename from docs/changes/unreleased/0-app-devices.md rename to docs/changes/unreleased/1977-app-devices.md index 426d3d73d1..a9b0032f42 100644 --- a/docs/changes/unreleased/0-app-devices.md +++ b/docs/changes/unreleased/1977-app-devices.md @@ -1,7 +1,7 @@ --- kind: added title: Desktop engines can list, rename and remove paired computers -pr: 0 +pr: 1977 surface: [engine] invalidates: - "The app engine had no device-management methods. Engines that advertise devices.manage now serve devices.list, devices.rename and devices.remove." From 1ce28a957e719e7e3e2a516ee0e7efcf549e4d79 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 15:58:23 -0400 Subject: [PATCH 6/7] test(app): model an unreachable relay and wait out the presence debounce TestAppDevicesV1V3OlderRosterCannotUndoRemoval stood in for a relay outage with a 503, which the engine rightly reports as an ordinary directory error; close the test server instead so the refresh is a transport failure, and require the engine's unreachable sentence. TestAppDevicesV1V5Existing- WatchSuppliesPresence waited five seconds for the offline announcement that the relay debounces for fifteen (OfflineDebounce); wait past the debounce. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/codeaf/appdevices_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cmd/codeaf/appdevices_test.go b/cmd/codeaf/appdevices_test.go index 084d2413f9..62dd0cb132 100644 --- a/cmd/codeaf/appdevices_test.go +++ b/cmd/codeaf/appdevices_test.go @@ -144,7 +144,7 @@ func (d *appDeviceDelayedList) wait(t *testing.T) { func TestAppDevicesV1V3OlderRosterCannotUndoRemoval(t *testing.T) { celltest.Available(t) delay := &appDeviceDelayedList{ready: make(chan struct{}), release: make(chan struct{})} - _, _, down := appDevicesRelay(t, delay.wrap) + srv, _, down := appDevicesRelay(t, delay.wrap) t.Cleanup(delay.resume) a, b := t.TempDir(), t.TempDir() id, err := identity.Ensure(a) @@ -192,7 +192,9 @@ func TestAppDevicesV1V3OlderRosterCannotUndoRemoval(t *testing.T) { case <-ctx.Done(): t.Fatal("the delayed roster response did not finish") } - if _, err := w.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeUnreachable { + // A 503 is a relay response; closing it makes the refresh unreachable. + srv.Close() + if _, err := w.call("devices.list", struct{}{}); err == nil || err.Code != appwire.CodeUnreachable || err.Message != chatlist.Unreachable { t.Fatalf("follow-up refresh must fail: %v", err) } // An offline rename publishes the retained roster through the real app @@ -236,7 +238,7 @@ func TestAppDevicesV1V5ExistingWatchSuppliesPresence(t *testing.T) { t.Fatal("the real directory has no follower") } t.Cleanup(follow.Close) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), directory.OfflineDebounce+10*time.Second) defer cancel() for !follow.State().Up { select { @@ -254,7 +256,12 @@ func TestAppDevicesV1V5ExistingWatchSuppliesPresence(t *testing.T) { return len(v.Devices) == 2 && v.Devices[1].Online != nil && *v.Devices[1].Online }) remote.Close() - appDeviceEvent(t, rec, func(v appwire.Devices) bool { + // The real relay announces offline only after its reconnect debounce. + rec.waitFor(t, directory.OfflineDebounce+5*time.Second, func(event string, data any) bool { + if event != appwire.EventDevicesChanged { + return false + } + v := data.(appwire.DevicesResult).Devices return len(v.Devices) == 2 && v.Devices[1].Online != nil && !*v.Devices[1].Online && !v.Devices[1].LastSeen.IsZero() }) rec.mu.Lock() From a1692cf93e9e86fb088e5356d6c733d345bb19e1 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 16:27:16 -0400 Subject: [PATCH 7/7] docs(atlas): point the devices node at the shared rename The device rename moved from cmd/codeaf/devrename.go's tellFleet into syncsetup.RenameDevice so the app and the CLI share it; the pairing atlas still named tellFleet, which TestEveryFilePathExists refuses. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/atlas/pairing.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/atlas/pairing.go b/internal/atlas/pairing.go index fe532759e9..8e98a458fe 100644 --- a/internal/atlas/pairing.go +++ b/internal/atlas/pairing.go @@ -79,7 +79,8 @@ var Pairing = &Map{ Summary: "`codeaf devices` and /devices list devices and show which are online. You can revoke or rename one. The relay refuses a revoked device, and the other devices hear about it through the directory watch.", Files: []FileRef{ {Path: "cmd/codeaf/devices.go", Symbols: []string{"runDevices", "listDevices", "stopDevice"}}, - {Path: "cmd/codeaf/devrename.go", Symbols: []string{"renameThisDevice", "tellFleet"}}, + {Path: "cmd/codeaf/devrename.go", Symbols: []string{"renameThisDevice"}}, + {Path: "internal/syncsetup/devices.go", Symbols: []string{"RenameDevice", "ObserveDevices"}}, {Path: "internal/directory/presence.go"}, }}, {ID: "sync", Label: "sync", Short: "syncsetup", Kind: KindGo, X: 0.55, Y: 0.38,