From 24eb1337de6ce143b7bfa46238cb3c4e12940af4 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Wed, 3 Jun 2026 11:18:30 +1200 Subject: [PATCH 1/3] Remember connections across refreshes --- internal/overlord/ifacestate/handlers.go | 107 +++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/internal/overlord/ifacestate/handlers.go b/internal/overlord/ifacestate/handlers.go index 297b5f040..957444fb4 100644 --- a/internal/overlord/ifacestate/handlers.go +++ b/internal/overlord/ifacestate/handlers.go @@ -201,11 +201,92 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, return err } + // Retrieve previously stored connections from the disconnect phase + var prevConns map[string]map[string]*schema.ConnState + chg := task.Change() + if err := chg.Get("prev-conns", &prevConns); err != nil && !errors.Is(err, state.ErrNoState) { + return err + } + var connectRefs = []*interfaces.ConnRef{} var wconns = workshopConns(wp) var plugDynamic = make(map[string]map[string]any) var slotDynamic = make(map[string]map[string]any) + // Restore previously connected interfaces that were disconnected during refresh + for _, sdkConns := range prevConns { + for connID, connState := range sdkConns { + // Only restore manually created connections (auto=false) that were + // not previously marked as undesired. Auto-connected pairs will be + // re-discovered by the auto-connect logic. + if connState.Auto || connState.Undesired { + continue + } + + connRef, err := interfaces.ParseConnRef(connID) + if err != nil { + continue + } + + // Skip if connection already exists in conns state + if _, ok := conns[connID]; ok { + continue + } + + // Only restore connections for the current SDK + if connRef.PlugRef.Sdk != info.Name && connRef.SlotRef.Sdk != info.Name { + continue + } + + // Check if plug and slot still exist in the repository + plug := m.repo.Plug(connRef.PlugRef.ProjectId, connRef.PlugRef.Workshop, connRef.PlugRef.Sdk, connRef.PlugRef.Name) + slot := m.repo.Slot(connRef.SlotRef.ProjectId, connRef.SlotRef.Workshop, connRef.SlotRef.Sdk, connRef.SlotRef.Name) + if plug == nil || slot == nil { + continue + } + + // Restore dynamic attributes from remounts for mount interfaces + if connState.Interface == "mount" { + if src, ok := remounts[connID]; ok { + if slotDynamic[connID] == nil { + slotDynamic[connID] = make(map[string]any) + } + slotDynamic[connID]["host-source"] = src + } + } + + // Restore dynamic attributes from the previous connection state + if connState.DynamicPlugAttrs != nil { + if plugDynamic[connID] == nil { + plugDynamic[connID] = make(map[string]any) + } + for k, v := range connState.DynamicPlugAttrs { + plugDynamic[connID][k] = v + } + } + if connState.DynamicSlotAttrs != nil { + if slotDynamic[connID] == nil { + slotDynamic[connID] = make(map[string]any) + } + for k, v := range connState.DynamicSlotAttrs { + slotDynamic[connID][k] = v + } + } + + connectRefs = append(connectRefs, connRef) + + // Mark this connection as restored so it won't be processed again + conns[connID] = &schema.ConnState{ + Auto: connState.Auto, + Interface: connState.Interface, + StaticPlugAttrs: connState.StaticPlugAttrs, + DynamicPlugAttrs: connState.DynamicPlugAttrs, + StaticSlotAttrs: connState.StaticSlotAttrs, + DynamicSlotAttrs: connState.DynamicSlotAttrs, + } + } + } + for _, plug := range info.Plugs { ref := plug.Ref() master, slaves := MaybeBound(wp, ref) @@ -391,6 +472,32 @@ func (m *InterfaceManager) doDisconnectInterfaces(task *state.Task, tomb *tomb.T return err } + // Store connection states before disconnect so they can be restored during refresh. + conns, err := getConns(st) + if err != nil { + return err + } + var prevConns map[string]*schema.ConnState + for _, cref := range connections { + if connState, ok := conns[cref.ID()]; ok { + if prevConns == nil { + prevConns = make(map[string]*schema.ConnState) + } + prevConns[cref.ID()] = connState + } + } + if prevConns != nil { + chg := task.Change() + var storedConns map[string]map[string]*schema.ConnState + if err = chg.Get("prev-conns", &storedConns); errors.Is(err, state.ErrNoState) { + storedConns = make(map[string]map[string]*schema.ConnState) + } else if err != nil { + return err + } + storedConns[s] = prevConns + chg.Set("prev-conns", storedConns) + } + ts := m.batchDisconnectTasks(*project, w, s, connections) handlersetup.InjectTasks(task, ts) From cf759c2b2dd40c383d4759702e4a87120be74c19 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 3 Jun 2026 02:48:50 +0000 Subject: [PATCH 2/3] fix: use maps.Copy for restored attrs --- internal/overlord/ifacestate/handlers.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/overlord/ifacestate/handlers.go b/internal/overlord/ifacestate/handlers.go index 957444fb4..4263916ca 100644 --- a/internal/overlord/ifacestate/handlers.go +++ b/internal/overlord/ifacestate/handlers.go @@ -260,17 +260,13 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, if plugDynamic[connID] == nil { plugDynamic[connID] = make(map[string]any) } - for k, v := range connState.DynamicPlugAttrs { - plugDynamic[connID][k] = v - } + maps.Copy(plugDynamic[connID], connState.DynamicPlugAttrs) } if connState.DynamicSlotAttrs != nil { if slotDynamic[connID] == nil { slotDynamic[connID] = make(map[string]any) } - for k, v := range connState.DynamicSlotAttrs { - slotDynamic[connID][k] = v - } + maps.Copy(slotDynamic[connID], connState.DynamicSlotAttrs) } connectRefs = append(connectRefs, connRef) From a30f2aae9d2c9f599fd9afee0a379c99712413ed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 3 Jun 2026 03:33:51 +0000 Subject: [PATCH 3/3] test: cover remembered refresh connections --- internal/overlord/ifacestate/handlers.go | 12 +- internal/overlord/ifacestate/handlers_test.go | 120 ++++++++++++++++++ 2 files changed, 129 insertions(+), 3 deletions(-) diff --git a/internal/overlord/ifacestate/handlers.go b/internal/overlord/ifacestate/handlers.go index 4263916ca..58bd393d5 100644 --- a/internal/overlord/ifacestate/handlers.go +++ b/internal/overlord/ifacestate/handlers.go @@ -140,7 +140,7 @@ func (m *InterfaceManager) remountSources(projectId, w, s string) map[string]str return candidates } -func (m *InterfaceManager) batchAutoConnectTasks(wp *workshop.Workshop, info *sdk.Info, refs []*interfaces.ConnRef, plugDynamic, slotDynamic map[string]map[string]any) *state.TaskSet { +func (m *InterfaceManager) batchAutoConnectTasks(wp *workshop.Workshop, info *sdk.Info, refs []*interfaces.ConnRef, autoConns map[string]bool, plugDynamic, slotDynamic map[string]map[string]any) *state.TaskSet { connectTs := state.NewTaskSet() var affected = map[sdk.Ref]bool{} @@ -149,7 +149,7 @@ func (m *InterfaceManager) batchAutoConnectTasks(wp *workshop.Workshop, info *sd connect.Set("plug", ref.PlugRef) connect.Set("slot", ref.SlotRef) - connect.Set("auto", true) + connect.Set("auto", autoConns[ref.ID()]) connect.Set("delayed-setup-profile", true) if plugDynamic != nil { @@ -209,6 +209,7 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, } var connectRefs = []*interfaces.ConnRef{} + var autoConns = make(map[string]bool) var wconns = workshopConns(wp) var plugDynamic = make(map[string]map[string]any) var slotDynamic = make(map[string]map[string]any) @@ -270,6 +271,7 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, } connectRefs = append(connectRefs, connRef) + autoConns[connID] = connState.Auto // Mark this connection as restored so it won't be processed again conns[connID] = &schema.ConnState{ @@ -316,6 +318,7 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, slref := &interfaces.ConnRef{PlugRef: slave, SlotRef: slotRef} if _, ok := conns[slref.ID()]; !ok { connectRefs = append(connectRefs, slref) + autoConns[slref.ID()] = true plugDynamic[slref.ID()] = make(map[string]any) plugDynamic[slref.ID()]["bind"] = connRef.ID() } @@ -328,6 +331,7 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, continue } connectRefs = append(connectRefs, connRef) + autoConns[connRef.ID()] = true } } @@ -364,6 +368,7 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, slref := &interfaces.ConnRef{PlugRef: slave, SlotRef: slotRef} if _, ok := conns[slref.ID()]; !ok { connectRefs = append(connectRefs, slref) + autoConns[slref.ID()] = true plugDynamic[slref.ID()] = make(map[string]any) plugDynamic[slref.ID()]["bind"] = connRef.ID() } @@ -376,6 +381,7 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, continue } connectRefs = append(connectRefs, connRef) + autoConns[connRef.ID()] = true } } @@ -389,7 +395,7 @@ func (m *InterfaceManager) connectAuto(task *state.Task, wp *workshop.Workshop, return a.ID() == b.ID() }) - ts := m.batchAutoConnectTasks(wp, info, connectRefs, plugDynamic, slotDynamic) + ts := m.batchAutoConnectTasks(wp, info, connectRefs, autoConns, plugDynamic, slotDynamic) handlersetup.InjectTasks(task, ts) m.state.EnsureBefore(0) task.SetStatus(state.DoneStatus) diff --git a/internal/overlord/ifacestate/handlers_test.go b/internal/overlord/ifacestate/handlers_test.go index 1fef6d11a..e8745ab0a 100644 --- a/internal/overlord/ifacestate/handlers_test.go +++ b/internal/overlord/ifacestate/handlers_test.go @@ -666,6 +666,70 @@ func (s *interfaceHandlersSuite) TestAutoconnectRemountedPlugsOnRefresh(c *check c.Assert(s.secBackend.RemoveCalls, check.HasLen, 0) } +func (s *interfaceHandlersSuite) TestAutoconnectRestoresPreviousConnectionsOnRefresh(c *check.C) { + // Setup + repo := s.mgr.Repository() + s.launchWorkshop(c, "ws-producer", []sdk.Meta{producer}) + c.Assert(repo.AddSdk(sdk.MockInfo(c, producer.SdkYAML, s.prj.ProjectId, "ws-producer")), check.IsNil) + + s.launchWorkshop(c, "ws", []sdk.Meta{consumer}) + c.Assert(repo.AddSdk(sdk.MockInfo(c, consumer.SdkYAML, s.prj.ProjectId, "ws")), check.IsNil) + + connRefKey := "42424242/ws/consumer:plug 42424242/ws-producer/producer:slot" + + // Execute + s.state.Lock() + chg := s.state.NewChange("refresh", "...") + chg.Set("prev-conns", map[string]map[string]*schema.ConnState{ + "consumer": { + connRefKey: { + Auto: false, + Interface: "mock-network", + StaticPlugAttrs: map[string]any{"attribute": "one"}, + DynamicPlugAttrs: map[string]any{"test-dynamic-attr": "new-dynamic-value"}, + }, + }, + }) + t1 := s.state.NewTask("resolve-interfaces", "...") + t2 := s.state.NewTask("auto-connect", "...") + t2.Set("sdk", "consumer") + t2.WaitFor(t1) + setWorkshopProject("ws", s.prj, t1, t2) + chg.Set("user", "testuser") + chg.AddTask(t1) + chg.AddTask(t2) + s.state.Unlock() + + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + c.Check(chg.Err(), check.IsNil) + + // Validate + ref, err := repo.Connected(s.prj.ProjectId, "ws", "consumer", "plug") + c.Assert(ref, check.HasLen, 1) + c.Assert(err, check.IsNil) + + ref, err = repo.Connected(s.prj.ProjectId, "ws-producer", "producer", "slot") + c.Assert(ref, check.HasLen, 1) + c.Assert(err, check.IsNil) + + var conns map[string]*schema.ConnState + c.Assert(s.state.Get("conns", &conns), check.IsNil) + c.Assert(conns, check.DeepEquals, map[string]*schema.ConnState{ + connRefKey: { + Auto: false, + Interface: "mock-network", + StaticPlugAttrs: map[string]any{"attribute": "one"}, + DynamicPlugAttrs: map[string]any{"test-dynamic-attr": "new-dynamic-value"}, + }, + }) + + c.Assert(s.secBackend.SetupCalls, check.HasLen, 2) + c.Assert(s.secBackend.RemoveCalls, check.HasLen, 0) +} + func (s *interfaceHandlersSuite) TestUndoAutoConnect(c *check.C) { // Setup // Create an already installed workshop with a candidate SDK/slot @@ -1159,6 +1223,62 @@ func (s *interfaceHandlersSuite) TestAutoDisconnectSavesRemounts(c *check.C) { c.Assert(s.secBackend.RemoveCalls, check.HasLen, 1) } +func (s *interfaceHandlersSuite) TestAutoDisconnectSavesPreviousConnections(c *check.C) { + // Setup + repo := s.mgr.Repository() + s.launchWorkshop(c, "ws-consumer", []sdk.Meta{consumer, producer}) + c.Assert(repo.AddSdk(sdk.MockInfo(c, consumer.SdkYAML, s.prj.ProjectId, "ws-consumer")), check.IsNil) + c.Assert(repo.AddSdk(sdk.MockInfo(c, producer.SdkYAML, s.prj.ProjectId, "ws-consumer")), check.IsNil) + + connRef := &interfaces.ConnRef{ + PlugRef: sdk.PlugRef{ProjectId: "42424242", Workshop: "ws-consumer", Sdk: "consumer", Name: "plug"}, + SlotRef: sdk.SlotRef{ProjectId: "42424242", Workshop: "ws-consumer", Sdk: "producer", Name: "slot"}, + } + + s.state.Lock() + s.state.Set("conns", map[string]any{ + connRef.ID(): map[string]any{ + "interface": "mock-network", + "auto": false, + "plug-static": map[string]any{"attribute": "one"}, + "plug-dynamic": map[string]any{"test-dynamic-attr": "new-dynamic-value"}, + }, + }) + _, err := repo.Connect(connRef, map[string]any{"attribute": "one"}, map[string]any{"test-dynamic-attr": "new-dynamic-value"}, nil, nil, nil) + c.Assert(err, check.IsNil) + _, err = ifacestate.ReloadConnections(s.mgr, "", "", "") + c.Assert(err, check.IsNil) + chg := s.newDisconnectInterfacesChange("consumer") + s.state.Unlock() + + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + c.Check(chg.Err(), check.IsNil) + + // Validate + conns, err := repo.Connections(s.prj.ProjectId, "ws-consumer", "consumer") + c.Assert(err, check.IsNil) + c.Assert(conns, check.HasLen, 0) + + var prevConns map[string]map[string]*schema.ConnState + c.Assert(chg.Get("prev-conns", &prevConns), check.IsNil) + c.Assert(prevConns, check.DeepEquals, map[string]map[string]*schema.ConnState{ + "consumer": { + connRef.ID(): { + Auto: false, + Interface: "mock-network", + StaticPlugAttrs: map[string]any{"attribute": "one"}, + DynamicPlugAttrs: map[string]any{"test-dynamic-attr": "new-dynamic-value"}, + }, + }, + }) + + c.Assert(s.secBackend.SetupCalls, check.HasLen, 2) + c.Assert(s.secBackend.RemoveCalls, check.HasLen, 1) +} + func (s *interfaceHandlersSuite) TestAutoDisconnectIgnoresAutoConnections(c *check.C) { // Setup // Create an already installed workshop with an auto-connected mount plug