Fix route changes and diagnostic scheduling

internal/api/tools_handlers.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -17,16 +18,22 @@ func (s *Server) handleToolsPing(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Bound the overall diagnostic, including DNS and process startup.
 	timeoutSec := req.Timeout
 	if timeoutSec <= 0 {
-		timeoutSec = 10
+		timeoutSec = 5
 	}
-	ctx, cancel := r.Context(), func() {}
-	if timeoutSec > 0 {
-		ctx, cancel = r.Context(), func() {}
-		// Allow up to 35 seconds for ping
-		_ = time.Duration(timeoutSec) * time.Second
+	if timeoutSec > 30 {
+		timeoutSec = 30
 	}
+	count := req.Count
+	if count <= 0 {
+		count = 4
+	}
+	if count > 30 {
+		count = 30
+	}
+	ctx, cancel := context.WithTimeout(r.Context(), time.Duration(timeoutSec+count+5)*time.Second)
 	defer cancel()
 
 	resp, err := s.toolsMgr.RunPing(ctx, req)

internal/dynamicrouting/bfd_transaction_test.go

@@ -64,3 +64,53 @@ func TestActiveEIGRPRequiresVtysh(t *testing.T) {
 		t.Fatalf("inactive EIGRP: %v", err)
 	}
 }
+
+func TestRoutingFilterApplyFailureRestoresPreviousConfig(t *testing.T) {
+	root := t.TempDir()
+	if err := os.Mkdir(filepath.Join(root, "bird.conf.new"), 0700); err != nil {
+		t.Fatal(err)
+	}
+	path := filepath.Join(root, "dynamic.json")
+	old := RoutingFilterRule{ID: "rule-1", Chain: "test", Action: "accept", RuleOrder: 10}
+	m := &Manager{configPath: path, birdConfigPath: filepath.Join(root, "bird.conf"), config: DynamicConfig{RoutingFilters: []RoutingFilterRule{old}}}
+	if err := m.save(); err != nil {
+		t.Fatal(err)
+	}
+	edited := old
+	edited.Action = "discard"
+	if err := m.SaveRoutingFilter(&edited); err == nil || !strings.Contains(err.Error(), "applying routing policies") {
+		t.Fatalf("apply failure: %v", err)
+	}
+	rules := m.ListRoutingFilters()
+	if len(rules) != 1 || rules[0].Action != "accept" {
+		t.Fatalf("memory not restored: %+v", rules)
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var stored DynamicConfig
+	if err = json.Unmarshal(data, &stored); err != nil {
+		t.Fatal(err)
+	}
+	if len(stored.RoutingFilters) != 1 || stored.RoutingFilters[0].Action != "accept" {
+		t.Fatalf("storage not restored: %+v", stored.RoutingFilters)
+	}
+}
+func TestRoutingFilterSaveFailureRestoresMemory(t *testing.T) {
+	root := t.TempDir()
+	bad := filepath.Join(root, "dir")
+	if err := os.Mkdir(bad, 0700); err != nil {
+		t.Fatal(err)
+	}
+	old := RoutingFilterRule{ID: "rule", Chain: "x", Action: "accept"}
+	m := &Manager{configPath: bad, config: DynamicConfig{RoutingFilters: []RoutingFilterRule{old}}}
+	next := old
+	next.Action = "discard"
+	if err := m.SaveRoutingFilter(&next); err == nil {
+		t.Fatal("save failure ignored")
+	}
+	if got := m.ListRoutingFilters(); len(got) != 1 || got[0].Action != "accept" {
+		t.Fatalf("memory changed: %+v", got)
+	}
+}

internal/dynamicrouting/manager.go

@@ -2588,8 +2588,38 @@ func (m *Manager) ListRoutingFilters() []RoutingFilterRule {
 	return filters
 }
 
+// snapshotRoutingPolicies copies policy slices before a mutation while m.mu is held.
+func (m *Manager) snapshotRoutingPolicies() DynamicConfig {
+	previous := m.config
+	previous.RoutingFilters = append([]RoutingFilterRule(nil), m.config.RoutingFilters...)
+	previous.RoutingCommunityLists = append([]RoutingFilterCommunityList(nil), m.config.RoutingCommunityLists...)
+	previous.RoutingASLists = append([]RoutingFilterASList(nil), m.config.RoutingASLists...)
+	return previous
+}
+func (m *Manager) commitRoutingPolicies(previous DynamicConfig) error {
+	if err := m.save(); err != nil {
+		m.config = previous
+		return fmt.Errorf("saving routing policies: %w", err)
+	}
+	if err := m.applyBirdConfig(); err != nil {
+		m.config = previous
+		var failures []error
+		failures = append(failures, fmt.Errorf("applying routing policies: %w", err))
+		if restoreErr := m.save(); restoreErr != nil {
+			failures = append(failures, fmt.Errorf("restoring routing policies: %w", restoreErr))
+		}
+		if applyErr := m.applyBirdConfig(); applyErr != nil {
+			failures = append(failures, fmt.Errorf("reapplying previous routing policies: %w", applyErr))
+		}
+		return errors.Join(failures...)
+	}
+	return nil
+}
+
 func (m *Manager) SaveRoutingFilter(rule *RoutingFilterRule) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	if rule.ID == "" {
 		rule.ID = fmt.Sprintf("rf-%d", time.Now().UnixNano()/1e6)
 	}
@@ -2608,14 +2638,13 @@ func (m *Manager) SaveRoutingFilter(rule *RoutingFilterRule) error {
 	if !found {
 		m.config.RoutingFilters = append(m.config.RoutingFilters, *rule)
 	}
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 func (m *Manager) DeleteRoutingFilter(id string) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	newRules := make([]RoutingFilterRule, 0, len(m.config.RoutingFilters))
 	for _, r := range m.config.RoutingFilters {
 		if r.ID != id {
@@ -2623,29 +2652,26 @@ func (m *Manager) DeleteRoutingFilter(id string) error {
 		}
 	}
 	m.config.RoutingFilters = newRules
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 func (m *Manager) SetRoutingFilterState(id string, disabled bool) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	for i := range m.config.RoutingFilters {
 		if m.config.RoutingFilters[i].ID == id {
 			m.config.RoutingFilters[i].Disabled = disabled
 			break
 		}
 	}
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 func (m *Manager) MoveRoutingFilter(id string, direction string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 
 	var targetRule *RoutingFilterRule
 	for i := range m.config.RoutingFilters {
@@ -2702,7 +2728,10 @@ func (m *Manager) MoveRoutingFilter(id string, direction string) error {
 		for i, it := range items {
 			m.config.RoutingFilters[it.globalIndex].RuleOrder = (i + 1) * 10
 		}
-		_ = m.save()
+		if err := m.save(); err != nil {
+			m.config = previous
+			return fmt.Errorf("saving routing policies: %w", err)
+		}
 		return nil
 	}
 
@@ -2714,8 +2743,7 @@ func (m *Manager) MoveRoutingFilter(id string, direction string) error {
 		m.config.RoutingFilters[it.globalIndex].RuleOrder = (i + 1) * 10
 	}
 
-	_ = m.save()
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 // translateRos7RuleToBird converts a RouterOS v7 rule script (or fallback fields) to valid BIRD 2 syntax
@@ -2943,6 +2971,8 @@ func (m *Manager) ListCommunityLists() []RoutingFilterCommunityList {
 
 func (m *Manager) SaveCommunityList(cl *RoutingFilterCommunityList) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	if cl.ID == "" {
 		cl.ID = fmt.Sprintf("cl-%d", time.Now().UnixNano()/1e6)
 	}
@@ -2958,14 +2988,13 @@ func (m *Manager) SaveCommunityList(cl *RoutingFilterCommunityList) error {
 	if !found {
 		m.config.RoutingCommunityLists = append(m.config.RoutingCommunityLists, *cl)
 	}
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 func (m *Manager) DeleteCommunityList(id string) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	newLists := make([]RoutingFilterCommunityList, 0, len(m.config.RoutingCommunityLists))
 	for _, r := range m.config.RoutingCommunityLists {
 		if r.ID != id {
@@ -2973,24 +3002,20 @@ func (m *Manager) DeleteCommunityList(id string) error {
 		}
 	}
 	m.config.RoutingCommunityLists = newLists
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 func (m *Manager) SetCommunityListState(id string, disabled bool) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	for i := range m.config.RoutingCommunityLists {
 		if m.config.RoutingCommunityLists[i].ID == id {
 			m.config.RoutingCommunityLists[i].Disabled = disabled
 			break
 		}
 	}
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 // -------------------------------------------------------------
@@ -3008,6 +3033,8 @@ func (m *Manager) ListASLists() []RoutingFilterASList {
 
 func (m *Manager) SaveASList(al *RoutingFilterASList) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	if al.ID == "" {
 		al.ID = fmt.Sprintf("al-%d", time.Now().UnixNano()/1e6)
 	}
@@ -3023,14 +3050,13 @@ func (m *Manager) SaveASList(al *RoutingFilterASList) error {
 	if !found {
 		m.config.RoutingASLists = append(m.config.RoutingASLists, *al)
 	}
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 func (m *Manager) DeleteASList(id string) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	newLists := make([]RoutingFilterASList, 0, len(m.config.RoutingASLists))
 	for _, r := range m.config.RoutingASLists {
 		if r.ID != id {
@@ -3038,24 +3064,20 @@ func (m *Manager) DeleteASList(id string) error {
 		}
 	}
 	m.config.RoutingASLists = newLists
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 func (m *Manager) SetASListState(id string, disabled bool) error {
 	m.mu.Lock()
+	defer m.mu.Unlock()
+	previous := m.snapshotRoutingPolicies()
 	for i := range m.config.RoutingASLists {
 		if m.config.RoutingASLists[i].ID == id {
 			m.config.RoutingASLists[i].Disabled = disabled
 			break
 		}
 	}
-	_ = m.save()
-	m.mu.Unlock()
-
-	return m.applyBirdConfig()
+	return m.commitRoutingPolicies(previous)
 }
 
 // -------------------------------------------------------------

internal/dynamicrouting/protocols_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -634,6 +635,7 @@ func TestDynamicRoutingFiltersAllProtocols(t *testing.T) {
 
 func TestMoveRoutingFilter(t *testing.T) {
 	mgr := &Manager{
+		configPath: filepath.Join(t.TempDir(), "dynamic.json"),
 		config: DynamicConfig{
 			RoutingFilters: []RoutingFilterRule{
 				{ID: "r1", Chain: "chain-A", RuleOrder: 10, Rule: "dst in 10.1.0.0/16"},

internal/routing/manager.go

@@ -3,6 +3,7 @@ package routing
 import (
 	"bufio"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log"
 	"net"
@@ -766,7 +767,6 @@ func (m *RoutingManager) SaveRoute(r *RouteItem) (err error) {
 		if strings.HasPrefix(origID, "kr-") {
 			for _, lr := range m.listLiveRoutesLocked() {
 				if lr.ID == origID {
-					_ = m.deleteDirectKernelRoute(lr)
 					break
 				}
 			}
@@ -840,6 +840,7 @@ func (m *RoutingManager) DeleteRoute(id string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
+	before := append([]RouteItem(nil), m.staticRoutes...)
 	var updated []RouteItem
 	var target *RouteItem
 
@@ -855,6 +856,7 @@ func (m *RoutingManager) DeleteRoute(id string) error {
 
 	// 2. If not found by direct ID, check if it's a live kernel route ID (e.g. kr-...)
 	if target == nil {
+		updated = nil
 		for _, lr := range m.listLiveRoutesLocked() {
 			if lr.ID == id {
 				// Check if any static route corresponds to this live route
@@ -887,30 +889,47 @@ func (m *RoutingManager) DeleteRoute(id string) error {
 
 	if target != nil {
 		m.staticRoutes = updated
-		_ = m.save()
-		_ = m.syncRouteGroupToKernel(target.DstAddress, target.RoutingTable, target.Distance)
-		_ = m.deleteDirectKernelRoute(*target)
-		return nil
+		return m.commitManagedRouteChange(before, *target)
 	}
 
 	return fmt.Errorf("route not found")
 }
 
+// commitManagedRouteChange is called with the routing manager lock held.
+func (m *RoutingManager) commitManagedRouteChange(before []RouteItem, route RouteItem) error {
+	if err := m.save(); err != nil {
+		m.staticRoutes = before
+		return errors.Join(fmt.Errorf("saving route change: %w", err), m.save())
+	}
+	if err := m.syncRouteGroupToKernel(route.DstAddress, route.RoutingTable, route.Distance); err != nil {
+		m.staticRoutes = before
+		saveErr := m.save()
+		reapplyErr := m.syncRouteGroupToKernel(route.DstAddress, route.RoutingTable, route.Distance)
+		if saveErr != nil {
+			saveErr = fmt.Errorf("restoring stored routes: %w", saveErr)
+		}
+		if reapplyErr != nil {
+			reapplyErr = fmt.Errorf("reapplying previous route: %w", reapplyErr)
+		}
+		return errors.Join(fmt.Errorf("applying route change: %w", err), saveErr, reapplyErr)
+	}
+	return nil
+}
+
 func (m *RoutingManager) SetRouteState(id string, disabled bool) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	for i := range m.staticRoutes {
 		if m.staticRoutes[i].ID == id {
+			before := append([]RouteItem(nil), m.staticRoutes...)
 			m.staticRoutes[i].Disabled = disabled
 			if !disabled && m.staticRoutes[i].HealthStatus != "offline" {
 				m.staticRoutes[i].Active = true
 			} else if disabled {
 				m.staticRoutes[i].Active = false
 			}
-			_ = m.save()
-			_ = m.syncRouteGroupToKernel(m.staticRoutes[i].DstAddress, m.staticRoutes[i].RoutingTable, m.staticRoutes[i].Distance)
-			return nil
+			return m.commitManagedRouteChange(before, m.staticRoutes[i])
 		}
 	}
 
@@ -922,15 +941,14 @@ func (m *RoutingManager) SetRouteState(id string, disabled bool) error {
 			for i := range m.staticRoutes {
 				gwMatches := m.staticRoutes[i].Gateway == "" || lr.Gateway == m.staticRoutes[i].Gateway || lr.Interface == m.staticRoutes[i].Gateway || (m.staticRoutes[i].Interface != "" && lr.Interface == m.staticRoutes[i].Interface)
 				if normalizeDst(m.staticRoutes[i].DstAddress) == normLrDst && m.staticRoutes[i].RoutingTable == lr.RoutingTable && gwMatches {
+					before := append([]RouteItem(nil), m.staticRoutes...)
 					m.staticRoutes[i].Disabled = disabled
 					if !disabled && m.staticRoutes[i].HealthStatus != "offline" {
 						m.staticRoutes[i].Active = true
 					} else if disabled {
 						m.staticRoutes[i].Active = false
 					}
-					_ = m.save()
-					_ = m.syncRouteGroupToKernel(m.staticRoutes[i].DstAddress, m.staticRoutes[i].RoutingTable, m.staticRoutes[i].Distance)
-					return nil
+					return m.commitManagedRouteChange(before, m.staticRoutes[i])
 				}
 			}
 
@@ -940,9 +958,16 @@ func (m *RoutingManager) SetRouteState(id string, disabled bool) error {
 				lr.Static = true
 				lr.Active = false
 				lr.ID = fmt.Sprintf("route-%d", time.Now().UnixNano())
+				before := append([]RouteItem(nil), m.staticRoutes...)
 				m.staticRoutes = append(m.staticRoutes, lr)
-				_ = m.save()
-				_ = m.syncRouteGroupToKernel(lr.DstAddress, lr.RoutingTable, lr.Distance)
+				if err := m.save(); err != nil {
+					m.staticRoutes = before
+					return fmt.Errorf("saving adopted route: %w", err)
+				}
+				if err := m.syncRouteGroupToKernel(lr.DstAddress, lr.RoutingTable, lr.Distance); err != nil {
+					m.staticRoutes = before
+					return errors.Join(fmt.Errorf("disabling adopted route: %w", err), m.save())
+				}
 				return nil
 			}
 		}
@@ -1052,38 +1077,20 @@ func (m *RoutingManager) syncRouteGroupToKernel(dstAddress, routingTable string,
 	}
 
 	if len(activeRoutes) == 0 {
-		args := []string{"route", "del", normDst}
-		if distance > 0 {
-			args = append(args, "metric", strconv.Itoa(distance))
-		}
-		if tbl != "main" {
-			args = append(args, "table", tbl)
-		}
-		args = append(args, "proto", "static")
-		_ = exec.Command("ip", args...).Run()
-
-		// Also delete without proto static in case route protocol differed
-		fbArgs := []string{"route", "del", normDst}
-		if distance > 0 {
-			fbArgs = append(fbArgs, "metric", strconv.Itoa(distance))
-		}
-		if tbl != "main" {
-			fbArgs = append(fbArgs, "table", tbl)
-		}
-		_ = exec.Command("ip", fbArgs...).Run()
-
-		// Bare fallback
-		_ = exec.Command("ip", "route", "del", normDst).Run()
-
-		// Also delete blackhole route variant if it existed
-		bhArgs := []string{"route", "del", "blackhole", normDst}
-		if distance > 0 {
-			bhArgs = append(bhArgs, "metric", strconv.Itoa(distance))
-		}
-		if tbl != "main" {
-			bhArgs = append(bhArgs, "table", tbl)
+		// Only touch the requested static route group. Broad fallbacks can delete
+		// a route from another table, metric, or routing protocol.
+		for _, routeType := range []string{"", "blackhole"} {
+			args := []string{"route", "del"}
+			if routeType != "" {
+				args = append(args, routeType)
+			}
+			args = append(args, normDst, "metric", strconv.Itoa(distance), "table", tbl, "proto", "static")
+			out, err := exec.Command("ip", args...).CombinedOutput()
+			missing := strings.Contains(string(out), "No such process") || (tbl == "main" && strings.Contains(string(out), "FIB table does not exist"))
+			if err != nil && !missing {
+				return fmt.Errorf("failed to delete static route group (%s): %s: %w", strings.Join(args, " "), strings.TrimSpace(string(out)), err)
+			}
 		}
-		_ = exec.Command("ip", bhArgs...).Run()
 		return nil
 	}
 
@@ -1322,39 +1329,11 @@ func (m *RoutingManager) deleteDirectKernelRoute(r RouteItem) error {
 	if r.Distance > 0 {
 		args = append(args, "metric", strconv.Itoa(r.Distance))
 	}
-	cmd := exec.Command("ip", args...)
-	if out, err := cmd.CombinedOutput(); err == nil {
-		return nil
-	} else {
-		log.Printf("[RouteManager] direct ip route del (%s): %s (%v), attempting fallback", strings.Join(args, " "), string(out), err)
-	}
-
-	if isBlackhole {
-		fbBhArgs := []string{"route", "del", "blackhole", normDst}
-		if r.RoutingTable != "" && r.RoutingTable != "main" {
-			fbBhArgs = append(fbBhArgs, "table", r.RoutingTable)
-		}
-		if r.Distance > 0 {
-			fbBhArgs = append(fbBhArgs, "metric", strconv.Itoa(r.Distance))
-		}
-		if err := exec.Command("ip", fbBhArgs...).Run(); err == nil {
-			return nil
-		}
-		return exec.Command("ip", "route", "del", "blackhole", normDst).Run()
-	}
-
-	fbArgs := []string{"route", "del", normDst}
-	if r.RoutingTable != "" && r.RoutingTable != "main" {
-		fbArgs = append(fbArgs, "table", r.RoutingTable)
-	}
-	if r.Distance > 0 {
-		fbArgs = append(fbArgs, "metric", strconv.Itoa(r.Distance))
-	}
-	if err := exec.Command("ip", fbArgs...).Run(); err == nil {
-		return nil
+	out, err := exec.Command("ip", args...).CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("failed to delete route (%s): %s: %w", strings.Join(args, " "), strings.TrimSpace(string(out)), err)
 	}
-
-	return exec.Command("ip", "route", "del", normDst).Run()
+	return nil
 }
 
 // ==================== Policy Rules (/ip route rule) ====================

internal/routing/manager_test.go

@@ -432,8 +432,9 @@ func TestSDWANRouteGroupResolution(t *testing.T) {
 		t.Fatalf("expected Fibergrid via 100.68.0.1, got %s via %s", nhList[0].Interface, nhList[0].Gateway)
 	}
 
-	// 3. Mark primary as DOWN and verify automatic failover to wgx-Space
-	group.Members[0].Status = sdwan.StatusDown
+	// 3. Disable the primary and verify the backup is selected. Probe-driven
+	// DOWN transitions are covered by the SD-WAN health tests.
+	group.Members[0].Disabled = true
 	if err := sdwanMgr.Save(group); err != nil {
 		t.Fatalf("failed to update sdwan group: %v", err)
 	}

internal/routing/route_change_test.go

@@ -0,0 +1,112 @@
+package routing
+
+import (
+	"encoding/json"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func useTempRouteStores(t *testing.T) {
+	t.Helper()
+	oldStatic, oldPolicy, oldTables := StaticRoutesStorePath, PolicyRulesStorePath, CustomTablesStorePath
+	dir := t.TempDir()
+	StaticRoutesStorePath = filepath.Join(dir, "static.json")
+	PolicyRulesStorePath = filepath.Join(dir, "rules.json")
+	CustomTablesStorePath = filepath.Join(dir, "tables.json")
+	t.Cleanup(func() {
+		StaticRoutesStorePath, PolicyRulesStorePath, CustomTablesStorePath = oldStatic, oldPolicy, oldTables
+	})
+}
+func TestRouteEnableFailureRestoresStoredAndMemoryState(t *testing.T) {
+	useTempRouteStores(t)
+	bin := t.TempDir()
+	t.Setenv("PATH", bin)
+	script := "#!/bin/sh\nif [ \"$2\" = del ]; then echo 'RTNETLINK answers: No such process' >&2; exit 2; fi\necho 'permission denied' >&2; exit 2\n"
+	if err := os.WriteFile(filepath.Join(bin, "ip"), []byte(script), 0755); err != nil {
+		t.Fatal(err)
+	}
+	old := RouteItem{ID: "r1", DstAddress: "198.51.100.0/24", Gateway: "192.0.2.1", Interface: "eth0", RoutingTable: "main", Distance: 10, Disabled: true, Active: false}
+	m := &RoutingManager{staticRoutes: []RouteItem{old}}
+	if err := m.save(); err != nil {
+		t.Fatal(err)
+	}
+	err := m.SetRouteState("r1", false)
+	if err == nil || !strings.Contains(err.Error(), "applying route change") {
+		t.Fatalf("expected apply error: %v", err)
+	}
+	if !m.staticRoutes[0].Disabled || m.staticRoutes[0].Active {
+		t.Fatalf("memory not restored: %+v", m.staticRoutes[0])
+	}
+	data, err := os.ReadFile(StaticRoutesStorePath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var persisted []RouteItem
+	if err = json.Unmarshal(data, &persisted); err != nil {
+		t.Fatal(err)
+	}
+	if len(persisted) != 1 || !persisted[0].Disabled {
+		t.Fatalf("storage not restored: %+v", persisted)
+	}
+}
+func TestRouteDeletionNeverUsesUnqualifiedFallback(t *testing.T) {
+	useTempRouteStores(t)
+	bin := t.TempDir()
+	t.Setenv("PATH", bin)
+	logPath := filepath.Join(t.TempDir(), "commands")
+	t.Setenv("DEBTIK_TEST_IP_LOG", logPath)
+	script := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"$DEBTIK_TEST_IP_LOG\"\necho 'RTNETLINK answers: No such process' >&2\nexit 2\n"
+	if err := os.WriteFile(filepath.Join(bin, "ip"), []byte(script), 0755); err != nil {
+		t.Fatal(err)
+	}
+	r := RouteItem{ID: "r1", DstAddress: "198.51.100.0/24", Gateway: "192.0.2.1", Interface: "eth0", RoutingTable: "100", Distance: 10, Active: true}
+	m := &RoutingManager{staticRoutes: []RouteItem{r}}
+	if err := m.save(); err != nil {
+		t.Fatal(err)
+	}
+	if err := m.DeleteRoute("r1"); err != nil {
+		t.Fatal(err)
+	}
+	data, err := os.ReadFile(logPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	lines := strings.Split(strings.TrimSpace(string(data)), "\n")
+	if len(lines) != 2 {
+		t.Fatalf("unexpected commands: %s", data)
+	}
+	for _, line := range lines {
+		if !strings.Contains(line, "metric 10 table 100 proto static") {
+			t.Fatalf("unsafe deletion: %s", line)
+		}
+	}
+}
+
+func TestIsolatedRouteGroupDeletionKeepsOtherMetric(t *testing.T) {
+	if os.Getenv("DEBTIK_TEST_ISOLATED_NETNS") != "1" {
+		t.Skip("requires a dedicated network namespace")
+	}
+	ip := func(args ...string) {
+		t.Helper()
+		if out, err := exec.Command("ip", args...).CombinedOutput(); err != nil {
+			t.Fatalf("ip %v: %s: %v", args, out, err)
+		}
+	}
+	ip("link", "set", "lo", "up")
+	ip("route", "add", "198.51.100.0/24", "dev", "lo", "metric", "10", "proto", "static")
+	ip("route", "add", "198.51.100.0/24", "dev", "lo", "metric", "20", "proto", "static")
+	m := &RoutingManager{}
+	if err := m.syncRouteGroupToKernel("198.51.100.0/24", "main", 10); err != nil {
+		t.Fatal(err)
+	}
+	out, err := exec.Command("ip", "route", "show", "198.51.100.0/24").CombinedOutput()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(out), "metric 10") || !strings.Contains(string(out), "metric 20") {
+		t.Fatalf("unexpected routes: %s", out)
+	}
+}

internal/sdwan/health.go

@@ -49,15 +49,19 @@ func (h *HealthMonitor) Start(getGroups func() []*SDWANGroup) {
 
 	go func() {
 		defer close(h.done)
-		ticker := time.NewTicker(3 * time.Second)
+		ticker := time.NewTicker(time.Second)
 		defer ticker.Stop()
+		lastProbe := make(map[string]time.Time)
 
 		for {
 			select {
 			case <-ctx.Done():
 				return
 			case <-ticker.C:
-				groups := getGroups()
+				groups := dueProbeGroups(getGroups(), lastProbe, time.Now())
+				if len(groups) == 0 {
+					continue
+				}
 				changed := h.runProbeIteration(groups)
 				if ctx.Err() != nil {
 					return
@@ -73,6 +77,33 @@ func (h *HealthMonitor) Start(getGroups func() []*SDWANGroup) {
 	}()
 }
 
+// dueProbeGroups respects the configured interval for each group. New groups
+// are probed on the next tick; removed or disabled groups lose their schedule.
+func dueProbeGroups(groups []*SDWANGroup, last map[string]time.Time, now time.Time) []*SDWANGroup {
+	active := make(map[string]bool)
+	var due []*SDWANGroup
+	for _, group := range groups {
+		if group == nil || group.Disabled {
+			continue
+		}
+		active[group.ID] = true
+		interval := group.ProbeInterval
+		if interval <= 0 {
+			interval = 3
+		}
+		if previous, ok := last[group.ID]; !ok || now.Sub(previous) >= time.Duration(interval)*time.Second {
+			due = append(due, group)
+			last[group.ID] = now
+		}
+	}
+	for id := range last {
+		if !active[id] {
+			delete(last, id)
+		}
+	}
+	return due
+}
+
 // Stop terminates the probing goroutine.
 func (h *HealthMonitor) Stop() {
 	if h.cancel != nil {

internal/sdwan/health_concurrency_test.go

@@ -94,3 +94,25 @@ func TestFailoverProbeHysteresisAndRecovery(t *testing.T) {
 		t.Fatalf("primary not restored: %+v", hops)
 	}
 }
+
+func TestProbeIntervalsPerGroup(t *testing.T) {
+	now := time.Unix(1000, 0)
+	last := map[string]time.Time{}
+	groups := []*SDWANGroup{{ID: "fast", ProbeInterval: 3}, {ID: "slow", ProbeInterval: 10}, {ID: "disabled", Disabled: true}}
+	if due := dueProbeGroups(groups, last, now); len(due) != 2 {
+		t.Fatalf("initial probes: %d", len(due))
+	}
+	if due := dueProbeGroups(groups, last, now.Add(2*time.Second)); len(due) != 0 {
+		t.Fatalf("probed too early: %d", len(due))
+	}
+	if due := dueProbeGroups(groups, last, now.Add(3*time.Second)); len(due) != 1 || due[0].ID != "fast" {
+		t.Fatalf("3-second interval: %+v", due)
+	}
+	if due := dueProbeGroups(groups, last, now.Add(10*time.Second)); len(due) != 2 {
+		t.Fatalf("10-second interval: %+v", due)
+	}
+	dueProbeGroups(groups[:1], last, now.Add(11*time.Second))
+	if _, ok := last["slow"]; ok {
+		t.Fatal("removed group retained schedule")
+	}
+}

internal/tools/manager.go

@@ -3,6 +3,7 @@ package tools
 import (
 	"bufio"
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"os/exec"
@@ -76,7 +77,10 @@ func (m *Manager) RunPing(ctx context.Context, req PingRequest) (*PingResponse,
 		"-W", strconv.Itoa(timeout),
 	}
 
-	if req.Interface != "" && validHostRegex.MatchString(req.Interface) {
+	if req.Interface != "" {
+		if !validHostRegex.MatchString(req.Interface) {
+			return nil, fmt.Errorf("invalid interface name")
+		}
 		args = append(args, "-I", req.Interface)
 	}
 
@@ -87,8 +91,17 @@ func (m *Manager) RunPing(ctx context.Context, req PingRequest) (*PingResponse,
 	args = append(args, target)
 
 	cmd := exec.CommandContext(ctx, "ping", args...)
-	outBytes, _ := cmd.CombinedOutput()
+	outBytes, runErr := cmd.CombinedOutput()
 	outStr := string(outBytes)
+	if ctx.Err() != nil {
+		return nil, ctx.Err()
+	}
+	if runErr != nil {
+		var exitErr *exec.ExitError
+		if !errors.As(runErr, &exitErr) || exitErr.ExitCode() != 1 {
+			return nil, fmt.Errorf("ping failed: %s: %w", strings.TrimSpace(outStr), runErr)
+		}
+	}
 
 	resp := &PingResponse{
 		Target:      target,
@@ -177,7 +190,10 @@ func (m *Manager) StreamPing(ctx context.Context, req PingRequest, onEvent func(
 		args = append(args, "-c", strconv.Itoa(req.Count))
 	}
 
-	if req.Interface != "" && validHostRegex.MatchString(req.Interface) {
+	if req.Interface != "" {
+		if !validHostRegex.MatchString(req.Interface) {
+			return fmt.Errorf("invalid interface name")
+		}
 		args = append(args, "-I", req.Interface)
 	}
 
@@ -368,7 +384,20 @@ func (m *Manager) StreamPing(ctx context.Context, req PingRequest, onEvent func(
 		})
 	}
 
-	_ = cmd.Wait()
+	scanErr := scanner.Err()
+	waitErr := cmd.Wait()
+	if scanErr != nil {
+		return fmt.Errorf("reading ping output: %w", scanErr)
+	}
+	if ctx.Err() != nil {
+		return ctx.Err()
+	}
+	if waitErr != nil {
+		var exitErr *exec.ExitError
+		if !errors.As(waitErr, &exitErr) || exitErr.ExitCode() != 1 {
+			return fmt.Errorf("ping failed: %w", waitErr)
+		}
+	}
 
 	lossPct := 0.0
 	if packetsSent > 0 {

internal/tools/manager_test.go

@@ -0,0 +1,66 @@
+package tools
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestPingReportsExecutionErrorsAndNoReply(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("PATH", dir)
+	m := NewManager()
+	if _, err := m.RunPing(context.Background(), PingRequest{Target: "192.0.2.1"}); err == nil {
+		t.Fatal("missing ping binary reported as timeout")
+	}
+	path := filepath.Join(dir, "ping")
+	if err := os.WriteFile(path, []byte("#!/bin/sh\nprintf 'ping: invalid option\\n'\nexit 2\n"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := m.RunPing(context.Background(), PingRequest{Target: "192.0.2.1"}); err == nil || !strings.Contains(err.Error(), "invalid option") {
+		t.Fatalf("execution error: %v", err)
+	}
+	if err := os.WriteFile(path, []byte("#!/bin/sh\nprintf '1 packets transmitted, 0 received, 100%% packet loss\\n'\nexit 1\n"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	resp, err := m.RunPing(context.Background(), PingRequest{Target: "192.0.2.1", Count: 1})
+	if err != nil || resp.Status != "timeout" || resp.PacketLossPct != 100 {
+		t.Fatalf("no reply: %+v %v", resp, err)
+	}
+	if _, err := m.RunPing(context.Background(), PingRequest{Target: "192.0.2.1", Interface: "bad interface"}); err == nil {
+		t.Fatal("invalid interface ignored")
+	}
+}
+
+func TestStreamPingReportsExecutionError(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("PATH", dir)
+	if err := os.WriteFile(filepath.Join(dir, "ping"), []byte("#!/bin/sh\nprintf 'ping: invalid option\\n'\nexit 2\n"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	var events []PingStreamEvent
+	err := NewManager().StreamPing(context.Background(), PingRequest{Target: "192.0.2.1"}, func(e PingStreamEvent) { events = append(events, e) })
+	if err == nil {
+		t.Fatal("ping process failure ignored")
+	}
+	for _, event := range events {
+		if event.Type == "done" {
+			t.Fatal("reported completion after execution failure")
+		}
+	}
+	if err = NewManager().StreamPing(context.Background(), PingRequest{Target: "192.0.2.1", Interface: "bad interface"}, func(PingStreamEvent) {}); err == nil {
+		t.Fatal("invalid interface accepted")
+	}
+}
+
+func TestPingHonorsCanceledContext(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	_, err := NewManager().RunPing(ctx, PingRequest{Target: "192.0.2.1"})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("expected canceled context, got %v", err)
+	}
+}