feat: build new state machine nickKeeper

refactor: network accessors and nickkeeper own wiring
refactor: handlers now delegate to nickkeeper
feat: added testing

server/models/network.ts

@@ -12,6 +12,7 @@ import Client from "../client";
 import {MessageType} from "../../shared/types/msg";
 import {ChanType} from "../../shared/types/chan";
 import {SharedNetwork} from "../../shared/types/network";
+import NickKeeper, {NickKeeperOwner} from "./nickKeeper";
 
 type NetworkIrcOptions = {
 	host: string;
@@ -121,6 +122,7 @@ class Network {
 	chanCache!: Chan[];
 	ignoreList!: IgnoreList;
 	keepNick!: string | null;
+	private nickKeeper?: NickKeeper;
 
 	status!: NetworkStatus;
 
@@ -196,6 +198,32 @@ class Network {
 		);
 	}
 
+	getKeepNick(this: Network) {
+		return this.keepNick;
+	}
+
+	setKeepNick(this: Network, nick: string) {
+		this.keepNick = nick;
+	}
+
+	clearKeepNick(this: Network) {
+		this.keepNick = null;
+	}
+
+	getNickKeeper(this: Network) {
+		if (!this.nickKeeper) {
+			const owner: NickKeeperOwner = {
+				getKeepNick: () => this.getKeepNick(),
+				setKeepNick: (nick) => this.setKeepNick(nick),
+				clearKeepNick: () => this.clearKeepNick(),
+			};
+
+			this.nickKeeper = new NickKeeper(owner);
+		}
+
+		return this.nickKeeper;
+	}
+
 	validate(this: Network, client: Client) {
 		// Remove !, :, @ and whitespace characters from nicknames and usernames
 		const cleanNick = (str: string) => str.replace(/[\x00\s:!@]/g, "_").substring(0, 100);
@@ -394,7 +422,7 @@ class Network {
 		const oldNick = this.nick;
 		const oldRealname = this.realname;
 
-		this.keepNick = null;
+		this.clearKeepNick();
 		this.nick = args.nick;
 		this.host = String(args.host || "");
 		this.name = String(args.name || "") || this.host;
@@ -493,7 +521,7 @@ class Network {
 		);
 
 		if (this.keepNick === nick) {
-			this.keepNick = null;
+			this.clearKeepNick();
 		}
 
 		if (this.irc?.options) {

server/models/nickKeeper.ts

@@ -0,0 +1,60 @@
+export type NickInUseContext = {
+	registered: boolean;
+	isPublic: boolean;
+};
+
+export type RegisteredResult = {
+	shouldUpdatePreferred: boolean;
+};
+
+export type NickKeeperOwner = {
+	getKeepNick(): string | null;
+	setKeepNick(nick: string): void;
+	clearKeepNick(): void;
+};
+
+export default class NickKeeper {
+	private owner: NickKeeperOwner;
+
+	constructor(owner: NickKeeperOwner) {
+		this.owner = owner;
+	}
+
+	onNickInUse(preferredNick: string, context: NickInUseContext) {
+		if (!context.registered && !context.isPublic) {
+			this.owner.setKeepNick(preferredNick);
+		}
+	}
+
+	onRegistered(registeredNick: string, preferredNick: string): RegisteredResult {
+		if (registeredNick === preferredNick) {
+			if (this.owner.getKeepNick() === registeredNick) {
+				this.owner.clearKeepNick();
+			}
+
+			return {shouldUpdatePreferred: true};
+		}
+
+		return {shouldUpdatePreferred: false};
+	}
+
+	onQuit(quitNick: string) {
+		if (this.owner.getKeepNick() === quitNick) {
+			this.owner.clearKeepNick();
+			return true;
+		}
+
+		return false;
+	}
+
+	onSocketClose() {
+		const keepNick = this.owner.getKeepNick();
+
+		if (!keepNick) {
+			return null;
+		}
+
+		this.owner.clearKeepNick();
+		return keepNick;
+	}
+}

server/plugins/inputs/nick.ts

@@ -43,7 +43,7 @@ const input: PluginInputHandler = function (network, chan, cmd, args) {
 	}
 
 	// If we were trying to keep a nick and user changes nick, stop trying to keep the old one
-	network.keepNick = null;
+	network.clearKeepNick();
 
 	// If connected to IRC, send to server and wait for ACK
 	// otherwise update the nick and UI straight away

server/plugins/irc-events/connection.ts

@@ -124,12 +124,13 @@ export default <IrcEventHandler>function (irc, network) {
 			);
 		}
 
-		if (network.keepNick) {
-			// We disconnected without getting our original nick back yet, just set it back locally
-			irc.options.nick = irc.user.nick = network.keepNick;
+		const keepNick = network.getNickKeeper().onSocketClose();
 
-			network.setNick(network.keepNick);
-			network.keepNick = null;
+		if (keepNick) {
+			// We disconnected without getting our original nick back yet, just set it back locally
+			irc.options.nick = irc.user.nick = keepNick;
+
+			network.setNick(keepNick);
 
 			client.emit("nick", {
 				network: network.uuid,

server/plugins/irc-events/error.ts

@@ -40,9 +40,10 @@ export default <IrcEventHandler>function (irc, network) {
 		if (irc.connection.registered === false && !Config.values.public) {
 			message += " An attempt to use it will be made when this nick quits.";
 
-			// Store the user's preferred nick in keepNick so the quit handler can reclaim it
-			// This is the user's actual preference from network.nick, not a fallback
-			network.keepNick = network.nick;
+			// Store the user's preferred nick so the quit handler can reclaim it
+			network
+				.getNickKeeper()
+				.onNickInUse(network.nick, {registered: false, isPublic: Config.values.public});
 		}
 
 		const lobby = network.getLobby();

server/plugins/irc-events/quit.ts

@@ -27,9 +27,8 @@ export default <IrcEventHandler>function (irc, network) {
 		});
 
 		// If user with the nick we are trying to keep has quit, try to get this nick
-		if (network.keepNick === data.nick) {
-			irc.changeNick(network.keepNick);
-			network.keepNick = null;
+		if (network.getNickKeeper().onQuit(data.nick)) {
+			irc.changeNick(data.nick);
 		}
 	});
 };

server/plugins/irc-events/welcome.ts

@@ -6,15 +6,14 @@ export default <IrcEventHandler>function (irc, network) {
 	const client = this;
 
 	irc.on("registered", function (data) {
+		const nickKeeper = network.getNickKeeper();
+		const {shouldUpdatePreferred} = nickKeeper.onRegistered(data.nick, network.nick);
+
 		// Only update the user's preferred nick (network.nick) if we registered with it
 		// If we registered with a fallback nick (e.g., nick123), don't overwrite the preference
-		// This allows the existing quit handler to reclaim the preferred nick when available
-		if (data.nick === network.nick) {
-			// We got our preferred nick, clear keepNick if it was set
-			if (network.keepNick === data.nick) {
-				network.keepNick = null;
-			}
-		} else if (data.nick !== network.nick) {
+		if (shouldUpdatePreferred) {
+			network.setNick(data.nick);
+		} else {
 			// We registered with a fallback, don't call setNick which would overwrite the preference
 			// Just update the IRC session nick
 			irc.user.nick = data.nick;

test/tests/nickKeeper.ts

@@ -0,0 +1,112 @@
+import {expect} from "chai";
+import NickKeeper from "../../server/models/nickKeeper";
+
+type Owner = {
+	keepNick: string | null;
+	getKeepNick(): string | null;
+	setKeepNick(nick: string): void;
+	clearKeepNick(): void;
+};
+
+function createOwner(initial: string | null = null): Owner {
+	return {
+		keepNick: initial,
+		getKeepNick() {
+			return this.keepNick;
+		},
+		setKeepNick(nick: string) {
+			this.keepNick = nick;
+		},
+		clearKeepNick() {
+			this.keepNick = null;
+		},
+	};
+}
+
+describe("NickKeeper", function () {
+	it("stores preferred nick when nick in use on connect", function () {
+		const owner = createOwner();
+		const keeper = new NickKeeper(owner);
+
+		keeper.onNickInUse("preferred", {registered: false, isPublic: false});
+
+		expect(owner.keepNick).to.equal("preferred");
+	});
+
+	it("does not store preferred nick when already registered", function () {
+		const owner = createOwner();
+		const keeper = new NickKeeper(owner);
+
+		keeper.onNickInUse("preferred", {registered: true, isPublic: false});
+
+		expect(owner.keepNick).to.equal(null);
+	});
+
+	it("does not store preferred nick in public mode", function () {
+		const owner = createOwner();
+		const keeper = new NickKeeper(owner);
+
+		keeper.onNickInUse("preferred", {registered: false, isPublic: true});
+
+		expect(owner.keepNick).to.equal(null);
+	});
+
+	it("clears keepNick when registered with preferred nick", function () {
+		const owner = createOwner("preferred");
+		const keeper = new NickKeeper(owner);
+
+		const result = keeper.onRegistered("preferred", "preferred");
+
+		expect(result.shouldUpdatePreferred).to.equal(true);
+		expect(owner.keepNick).to.equal(null);
+	});
+
+	it("does not clear keepNick when registered with fallback", function () {
+		const owner = createOwner("preferred");
+		const keeper = new NickKeeper(owner);
+
+		const result = keeper.onRegistered("fallback", "preferred");
+
+		expect(result.shouldUpdatePreferred).to.equal(false);
+		expect(owner.keepNick).to.equal("preferred");
+	});
+
+	it("reclaims preferred nick on quit and clears keepNick", function () {
+		const owner = createOwner("preferred");
+		const keeper = new NickKeeper(owner);
+
+		const shouldReclaim = keeper.onQuit("preferred");
+
+		expect(shouldReclaim).to.equal(true);
+		expect(owner.keepNick).to.equal(null);
+	});
+
+	it("does not reclaim when quit nick does not match", function () {
+		const owner = createOwner("preferred");
+		const keeper = new NickKeeper(owner);
+
+		const shouldReclaim = keeper.onQuit("other");
+
+		expect(shouldReclaim).to.equal(false);
+		expect(owner.keepNick).to.equal("preferred");
+	});
+
+	it("returns keepNick on socket close and clears it", function () {
+		const owner = createOwner("preferred");
+		const keeper = new NickKeeper(owner);
+
+		const keepNick = keeper.onSocketClose();
+
+		expect(keepNick).to.equal("preferred");
+		expect(owner.keepNick).to.equal(null);
+	});
+
+	it("returns null on socket close when no keepNick", function () {
+		const owner = createOwner(null);
+		const keeper = new NickKeeper(owner);
+
+		const keepNick = keeper.onSocketClose();
+
+		expect(keepNick).to.equal(null);
+	});
+});