IRC: edit server settings as a draft, applied under the service lock

The IRC App edited the live config while the IRC task could be reading
it to connect. The App now edits a copy; applyConfig() validates it and
swaps it in under the lock, and the task connects from a snapshot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EhqxQ49eCju4CzKYNjZzwT

lib/irc/src/irc_config.h

@@ -22,6 +22,21 @@ class IrcConfig {
     std::string nickservPassword;        // otherwise IDENTIFY with NickServ, if set
     std::vector<std::string> autojoin;
 
+    // A copy is a draft the UI can edit freely; copySettingsFrom() applies one.
+    IrcConfig(const IrcConfig&) = default;
+    void copySettingsFrom(const IrcConfig& other) {
+        host = other.host;
+        port = other.port;
+        tls = other.tls;
+        allowSelfSigned = other.allowSelfSigned;
+        pinnedSha256 = other.pinnedSha256;
+        nick = other.nick;
+        saslUser = other.saslUser;
+        saslPassword = other.saslPassword;
+        nickservPassword = other.nickservPassword;
+        autojoin = other.autojoin;
+    }
+
     void load(const std::string& defaultNick);
     std::string save();  // empty on success, otherwise why it was refused
     std::string validate() const;

src/apps/irc_app.cpp

@@ -98,6 +98,7 @@ bool IrcApp::onChatKey(const KeyEvent& e) {
             input_.setText("");
             scroll_ = 0;
             if (lowered(text) == "/settings") {
+                draft_.reset(new IrcConfig(irc_.draftConfig()));
                 page_ = Page::Settings;
                 fields_.setCount(kFields);
                 break;
@@ -127,7 +128,7 @@ std::string IrcApp::fieldLabel(int f) const {
 }
 
 std::string IrcApp::fieldValue(int f) const {
-    const IrcConfig& c = irc_.config();
+    const IrcConfig& c = *draft_;
     auto secret = [](const std::string& s) { return s.empty() ? std::string("-") : std::string("(set)"); };
     switch (f) {
         case kHost: return c.host;
@@ -144,7 +145,7 @@ std::string IrcApp::fieldValue(int f) const {
 }
 
 void IrcApp::startEditing(int f) {
-    const IrcConfig& c = irc_.config();
+    const IrcConfig& c = *draft_;
     fieldEditor_ = LineEditor(f == kAutojoin ? 200 : 63);
     switch (f) {
         case kHost: fieldEditor_.setText(c.host); break;
@@ -158,7 +159,7 @@ void IrcApp::startEditing(int f) {
 }
 
 void IrcApp::finishEditing() {
-    IrcConfig& c = irc_.config();
+    IrcConfig& c = *draft_;
     const std::string& v = fieldEditor_.text();
     switch (fields_.selected()) {
         case kHost: c.host = v; break;
@@ -185,12 +186,12 @@ bool IrcApp::onSettingsKey(const KeyEvent& e) {
         }
         return true;
     }
-    IrcConfig& c = irc_.config();
+    IrcConfig& c = *draft_;
     switch (e.key) {
         case Key::Up: fields_.up(); break;
         case Key::Down: fields_.down(); break;
         case Key::Back:
-            c.load(c.nick);  // discard unsaved edits
+            draft_.reset();  // discard unsaved edits
             page_ = Page::Chat;
             break;
         case Key::Select:
@@ -201,7 +202,7 @@ bool IrcApp::onSettingsKey(const KeyEvent& e) {
                     c.pinnedSha256.clear();
                     break;
                 case kSave: {
-                    std::string error = irc_.saveConfig();
+                    std::string error = irc_.applyConfig(c);
                     if (!error.empty()) {
                         warn(error);
                         break;

src/apps/irc_app.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <memory>
 #include <string>
 
 #include "app.h"
@@ -50,6 +51,7 @@ class IrcApp : public App {
     ListModel fields_{theme::kContent.h / theme::kLineHeight};
     bool editing_ = false;
     LineEditor fieldEditor_{63};
+    std::unique_ptr<IrcConfig> draft_;  // the settings being edited, applied on Save
 };
 
 }  // namespace roro

src/services/irc_service.cpp

@@ -48,15 +48,19 @@ int IrcService::totalUnread() {
     return session_->totalUnread();
 }
 
-std::string IrcService::saveConfig() {
+IrcConfig IrcService::draftConfig() {
-    std::string error = config_.save();
-    if (!error.empty()) return error;
-    if (wanted_) {
-        restart_ = true;  // the task reconnects with a fresh session
-    } else {
     Lock l(lock_);
-        session_.reset(new IrcSession(config_));  // a new server or nick starts a fresh session
+    return config_;
 }
+
+std::string IrcService::applyConfig(const IrcConfig& draft) {
+    std::string error = draft.validate();
+    if (!error.empty()) return error;
+    Lock l(lock_);
+    config_.copySettingsFrom(draft);
+    config_.save();
+    if (wanted_) restart_ = true;  // the task reconnects with a fresh session
+    else session_.reset(new IrcSession(config_));
     return "";
 }
 
@@ -72,14 +76,15 @@ void IrcService::scheduleRetry(const std::string& why) {
 
 bool IrcService::open() {
     status_ = Status::Connecting;
-    bool pinning = config_.tls && config_.allowSelfSigned;
+    IrcConfig settings = draftConfig();  // a snapshot: the UI may apply new settings meanwhile
-    conn_ = config_.tls ? static_cast<NetworkClient*>(&tlsClient_) : &plainClient_;
+    bool pinning = settings.tls && settings.allowSelfSigned;
-    if (config_.tls) {
+    conn_ = settings.tls ? static_cast<NetworkClient*>(&tlsClient_) : &plainClient_;
+    if (settings.tls) {
         tlsClient_.setTimeout(15);  // seconds, for the handshake
         if (pinning) tlsClient_.setInsecure();
         else tlsClient_.useBuiltinCACertBundle();
     }
-    if (!conn_->connect(config_.host.c_str(), config_.port)) return false;
+    if (!conn_->connect(settings.host.c_str(), settings.port)) return false;
 
     if (pinning) {
         uint8_t sha[32];
@@ -88,10 +93,11 @@ bool IrcService::open() {
             return false;
         }
         std::string fingerprint = hex(sha, sizeof(sha));
-        if (config_.pinnedSha256.empty()) {
+        if (settings.pinnedSha256.empty()) {
+            Lock l(lock_);
             config_.pinnedSha256 = fingerprint;  // trust on first use
             config_.save();
-        } else if (config_.pinnedSha256 != fingerprint) {
+        } else if (settings.pinnedSha256 != fingerprint) {
             tlsClient_.stop();
             Lock l(lock_);
             session_->disconnected(clock_.utcNow(), "server certificate changed: not connecting");
@@ -145,6 +151,11 @@ void IrcService::flushEffects() {
         if (open_) conn_->print((line + "\r\n").c_str());
 
     std::string date = clock_.localDate();
+    std::string host;
+    {
+        Lock l(lock_);
+        host = config_.host;
+    }
     for (auto& entry : fx.logs) {
         const IrcLine& l = entry.line;
         std::string when = l.utc >= 0 ? ClockModel::formatLocalTime(l.utc) : "--:--";
@@ -156,7 +167,7 @@ void IrcService::flushEffects() {
             case IrcLine::Kind::Info: text = l.text; break;
             default: text = "<" + l.nick + "> " + l.text; break;
         }
-        storage_.appendLine(storage::dailyLogPath({"irc", config_.host, entry.buffer}, date), when + " " + text);
+        storage_.appendLine(storage::dailyLogPath({"irc", host, entry.buffer}, date), when + " " + text);
     }
     for (auto& n : fx.notifications)
         bus_.publish(Event::withText(EventType::Notification, n.c_str(), static_cast<int32_t>(NotificationLevel::Message)));
@@ -202,7 +213,7 @@ void IrcService::loop() {
             retryAtMs_ = now;  // reconnect as soon as Wi-Fi is back
         } else if (!open_) {
             if (static_cast<int32_t>(now - retryAtMs_) >= 0) {
-                if (!open()) scheduleRetry("could not connect to " + config_.host);
+                if (!open()) scheduleRetry("could not connect to " + draftConfig().host);
             }
         } else if (!conn_->connected()) {
             close("");

src/services/irc_service.h

@@ -44,9 +44,10 @@ class IrcService : public Service {
     }
     int totalUnread();
 
-    IrcConfig& config() { return config_; }
+    // A copy of the server settings to edit; applyConfig() validates, saves and, if running,
-    // Saves the config and, if running, reconnects with it (a fresh session).
+    // reconnects with it (a fresh session).
-    std::string saveConfig();
+    IrcConfig draftConfig();
+    std::string applyConfig(const IrcConfig& draft);
 
    private:
     struct Lock {

test/test_irc_config/test_irc_config.cpp

@@ -64,11 +64,25 @@ void test_autojoin_text_round_trip() {
     TEST_ASSERT_EQUAL_STRING("#a #b", IrcConfig::formatChannels({"#a", "#b"}).c_str());
 }
 
+void test_copy_settings_from_a_draft() {
+    MemoryStore store;
+    IrcConfig live(store);
+    live.load("x");
+    IrcConfig draft = live;
+    draft.host = "irc.example.org";
+    draft.autojoin = {"#a"};
+    TEST_ASSERT_EQUAL_STRING("irc.libera.chat", live.host.c_str());  // the draft is independent
+    live.copySettingsFrom(draft);
+    TEST_ASSERT_EQUAL_STRING("irc.example.org", live.host.c_str());
+    TEST_ASSERT_EQUAL(1, live.autojoin.size());
+}
+
 int main() {
     UNITY_BEGIN();
     RUN_TEST(test_defaults_point_at_libera_over_tls);
     RUN_TEST(test_save_and_reload);
     RUN_TEST(test_invalid_values_are_refused_with_a_reason);
     RUN_TEST(test_autojoin_text_round_trip);
+    RUN_TEST(test_copy_settings_from_a_draft);
     return UNITY_END();
 }