refactor(config): introduce Store interface, Remove method, and reset command

- Extract Store interface from concrete fileStore so the store is
  injectable and mockable without leaking the concrete type
- Add Remove(key) with atomic write and rollback, mirroring Set
- Add MQTT reset command to drop a persisted override (same allowlist
  as set); returns "not_configured" when no store is wired
- Add ApplyConfigEntryForTest export shim for white-box unit tests
- Add TestApplyConfigEntry covering all settable keys, invalid duration
  no-op, and unknown-key no-op
- Add TestStore_Remove_* covering existence, persistence, and write-error
  rollback; add reset cases to TestConfigGetSet

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

Author: JeffMboya

cmd/main.go

@@ -399,7 +399,7 @@ func initLogger(levelText string) (*slog.Logger, *slog.LevelVar, error) {
 	return slog.New(logHandler), &levelVar, nil
 }
 
-func applyPersistedOverrides(cfg agent.Config, store *pkgconfig.Store) agent.Config {
+func applyPersistedOverrides(cfg agent.Config, store pkgconfig.Store) agent.Config {
 	for key, val := range store.All() {
 		agent.ApplyConfigEntry(&cfg, key, val)
 	}

export_test.go

@@ -13,6 +13,10 @@ import (
 
 const NodeRedTLSConfigIDForTest = nodeRedTLSConfigID
 
+func ApplyConfigEntryForTest(cfg *Config, key, val string) {
+	ApplyConfigEntry(cfg, key, val)
+}
+
 func ChangeDirForTest(workDir string, cmd []string) (string, string, error) {
 	ag := &agent{workDir: workDir}
 	output, err := ag.changeDir(cmd)

pkg/config/store.go

@@ -10,9 +10,15 @@ import (
 	"sync"
 )
 
-// Store is a thread-safe, file-backed key-value store that persists entries
-// to a JSON file immediately on every Set call.
-type Store struct {
+// Store is the interface for a thread-safe, file-backed key-value store.
+type Store interface {
+	Get(key string) (string, bool)
+	Set(key, value string) error
+	Remove(key string) error
+	All() map[string]string
+}
+
+type fileStore struct {
 	mu      sync.RWMutex
 	entries map[string]string
 	path    string
@@ -21,8 +27,8 @@ type Store struct {
 // NewStore opens (or creates) the JSON file at path and loads any existing
 // entries into memory. Returns an error only if the file exists but cannot
 // be parsed; a missing file is treated as an empty store.
-func NewStore(path string) (*Store, error) {
-	s := &Store{
+func NewStore(path string) (Store, error) {
+	s := &fileStore{
 		entries: make(map[string]string),
 		path:    path,
 	}
@@ -33,7 +39,7 @@ func NewStore(path string) (*Store, error) {
 }
 
 // Get returns the value for key and whether it was found.
-func (s *Store) Get(key string) (string, bool) {
+func (s *fileStore) Get(key string) (string, bool) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 	v, ok := s.entries[key]
@@ -42,7 +48,7 @@ func (s *Store) Get(key string) (string, bool) {
 
 // Set writes key→value to the in-memory cache and immediately persists.
 // If the disk write fails the in-memory change is rolled back.
-func (s *Store) Set(key, value string) error {
+func (s *fileStore) Set(key, value string) error {
 	s.mu.Lock()
 	prev, existed := s.entries[key]
 	s.entries[key] = value
@@ -62,14 +68,36 @@ func (s *Store) Set(key, value string) error {
 	return nil
 }
 
+// Remove deletes key from the store and persists immediately.
+// It is a no-op if the key does not exist.
+func (s *fileStore) Remove(key string) error {
+	s.mu.Lock()
+	prev, existed := s.entries[key]
+	if !existed {
+		s.mu.Unlock()
+		return nil
+	}
+	delete(s.entries, key)
+	snapshot := s.copyEntries()
+	s.mu.Unlock()
+
+	if err := s.writeSnapshot(snapshot); err != nil {
+		s.mu.Lock()
+		s.entries[key] = prev
+		s.mu.Unlock()
+		return err
+	}
+	return nil
+}
+
 // All returns a snapshot of all entries.
-func (s *Store) All() map[string]string {
+func (s *fileStore) All() map[string]string {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 	return s.copyEntries()
 }
 
-func (s *Store) load() error {
+func (s *fileStore) load() error {
 	b, err := os.ReadFile(s.path)
 	if err != nil {
 		return err
@@ -81,13 +109,13 @@ func (s *Store) load() error {
 }
 
 // copyEntries returns a copy of s.entries. Must be called with the lock held.
-func (s *Store) copyEntries() map[string]string {
+func (s *fileStore) copyEntries() map[string]string {
 	out := make(map[string]string, len(s.entries))
 	maps.Copy(out, s.entries)
 	return out
 }
 
-func (s *Store) writeSnapshot(m map[string]string) error {
+func (s *fileStore) writeSnapshot(m map[string]string) error {
 	b, err := json.MarshalIndent(m, "", "  ")
 	if err != nil {
 		return err

pkg/config/store_test.go

@@ -93,6 +93,44 @@ func TestStore_All_Snapshot(t *testing.T) {
 	assert.False(t, ok, "snapshot mutation must not affect the store")
 }
 
+func TestStore_Remove_ExistingKey(t *testing.T) {
+	s, err := config.NewStore(filepath.Join(t.TempDir(), "config.json"))
+	require.NoError(t, err)
+	require.NoError(t, s.Set("key", "value"))
+	require.NoError(t, s.Remove("key"))
+	_, ok := s.Get("key")
+	assert.False(t, ok, "expected key to be removed")
+}
+
+func TestStore_Remove_MissingKey(t *testing.T) {
+	s, err := config.NewStore(filepath.Join(t.TempDir(), "config.json"))
+	require.NoError(t, err)
+	assert.NoError(t, s.Remove("nonexistent"), "remove of missing key must not error")
+}
+
+func TestStore_Remove_Persists(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "config.json")
+	s1, err := config.NewStore(path)
+	require.NoError(t, err)
+	require.NoError(t, s1.Set("key", "value"))
+	require.NoError(t, s1.Remove("key"))
+
+	s2, err := config.NewStore(path)
+	require.NoError(t, err)
+	_, ok := s2.Get("key")
+	assert.False(t, ok, "removed key must not reappear after reload")
+}
+
+func TestStore_Remove_WriteError(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "noexist", "config.json")
+	s, err := config.NewStore(path)
+	require.NoError(t, err)
+	// Manually seed an entry so Remove actually tries to write.
+	require.Error(t, s.Set("key", "val"), "expected write error")
+	// Remove on a missing-key store is a no-op — no write attempted.
+	assert.NoError(t, s.Remove("key"))
+}
+
 func TestStore_Set_WriteError(t *testing.T) {
 	// Path in a non-existent subdirectory — NewStore succeeds (file not found),
 	// but persist() fails because the parent directory doesn't exist.

service.go

@@ -137,13 +137,13 @@ type agent struct {
 	svcs                map[string]Heartbeat
 	terminals           map[string]terminal.Session
 	workDir             string
-	store               *cfgstore.Store
+	store               cfgstore.Store
 	heartbeatIntervalCh chan time.Duration
 	logLevel            *slog.LevelVar
 }
 
 // New returns agent service implementation.
-func New(ctx context.Context, mc paho.Client, cfg *Config, nc nodered.Client, logger *slog.Logger, store *cfgstore.Store, levelVar *slog.LevelVar) (Service, error) {
+func New(ctx context.Context, mc paho.Client, cfg *Config, nc nodered.Client, logger *slog.Logger, store cfgstore.Store, levelVar *slog.LevelVar) (Service, error) {
 	ag := &agent{
 		mqttClient:          mc,
 		noderedClient:       nc,
@@ -308,6 +308,22 @@ func (a *agent) ServiceConfig(ctx context.Context, uuid, cmdStr string) error {
 			a.applyLiveUpdate(key, val)
 			resp = "ok"
 		}
+	case "reset":
+		if len(cmdArgs) < 2 || cmdArgs[1] == "" {
+			return errInvalidCommand
+		}
+		key := cmdArgs[1]
+		if !settableKeys[key] {
+			return errInvalidCommand
+		}
+		if a.store == nil {
+			resp = "not_configured"
+		} else {
+			if err := a.store.Remove(key); err != nil {
+				return err
+			}
+			resp = "ok"
+		}
 	default:
 		return errInvalidCommand
 	}

service_test.go

@@ -49,7 +49,7 @@ func testConfig() agent.Config {
 	)
 }
 
-func newServiceWithStore(t *testing.T, cfg agent.Config, store *cfgstore.Store) (agent.Service, *agentmocks.MQTTClient, *nrmocks.Client, error) {
+func newServiceWithStore(t *testing.T, cfg agent.Config, store cfgstore.Store) (agent.Service, *agentmocks.MQTTClient, *nrmocks.Client, error) {
 	cfg.DomainID = domainID
 	mqttClient := agentmocks.NewMQTTClient(t)
 	nodeRed := nrmocks.NewClient(t)
@@ -514,11 +514,42 @@ func TestConfigGetSet(t *testing.T) {
 			useStore: true,
 			err:      true,
 		},
+		{
+			desc:     "reset key without store returns not_configured",
+			cmd:      "reset,log_level",
+			useStore: false,
+			wantResp: "not_configured",
+		},
+		{
+			desc:     "reset existing key returns ok",
+			cmd:      "reset,log_level",
+			useStore: true,
+			seed:     map[string]string{"log_level": "debug"},
+			wantResp: "ok",
+		},
+		{
+			desc:     "reset missing key is no-op and returns ok",
+			cmd:      "reset,log_level",
+			useStore: true,
+			wantResp: "ok",
+		},
+		{
+			desc:     "reject reset without key",
+			cmd:      "reset",
+			useStore: true,
+			err:      true,
+		},
+		{
+			desc:     "reject reset with unknown key",
+			cmd:      "reset,mqtt_password",
+			useStore: true,
+			err:      true,
+		},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.desc, func(t *testing.T) {
-			var s *cfgstore.Store
+			var s cfgstore.Store
 			if tc.useStore {
 				var storeErr error
 				s, storeErr = cfgstore.NewStore(filepath.Join(t.TempDir(), "config.json"))
@@ -547,6 +578,64 @@ func TestConfigGetSet(t *testing.T) {
 	}
 }
 
+func TestApplyConfigEntry(t *testing.T) {
+	cases := []struct {
+		desc    string
+		key     string
+		val     string
+		check   func(t *testing.T, cfg agent.Config)
+	}{
+		{
+			desc: "set log level",
+			key:  "log_level",
+			val:  "warn",
+			check: func(t *testing.T, cfg agent.Config) {
+				assert.Equal(t, "warn", cfg.Log.Level)
+			},
+		},
+		{
+			desc: "set heartbeat interval",
+			key:  "heartbeat_interval",
+			val:  "30s",
+			check: func(t *testing.T, cfg agent.Config) {
+				assert.Equal(t, 30*time.Second, cfg.Heartbeat.Interval)
+			},
+		},
+		{
+			desc: "set terminal session timeout",
+			key:  "terminal_session_timeout",
+			val:  "2m",
+			check: func(t *testing.T, cfg agent.Config) {
+				assert.Equal(t, 2*time.Minute, cfg.Terminal.SessionTimeout)
+			},
+		},
+		{
+			desc: "invalid duration is ignored",
+			key:  "heartbeat_interval",
+			val:  "not-a-duration",
+			check: func(t *testing.T, cfg agent.Config) {
+				assert.Equal(t, time.Hour, cfg.Heartbeat.Interval)
+			},
+		},
+		{
+			desc: "unknown key is a no-op",
+			key:  "mqtt_password",
+			val:  "secret",
+			check: func(t *testing.T, cfg agent.Config) {
+				assert.Equal(t, "client-secret", cfg.MQTT.Password)
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			cfg := testConfig()
+			agent.ApplyConfigEntryForTest(&cfg, tc.key, tc.val)
+			tc.check(t, cfg)
+		})
+	}
+}
+
 func TestTerminal(t *testing.T) {
 	cases := []struct {
 		desc      string