feat: implement IRC numerics batch 2 — connection registration, channel ops, user queries #59
clawbot
Reference in New Issue
Block a user
Delete Branch "feature/irc-numerics-batch2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Implements the remaining important/commonly-used IRC numeric reply codes, as requested in issue #52.
Connection Registration (001-005)
All sent automatically after RPL_WELCOME during session creation/login.
Server Statistics (251-255)
Sent during connection registration and available via LUSERS command.
Channel Operations
User Queries
Database Additions
GetChannelCount()— total channel count for LUSERSListAllChannelsWithCounts()— channels with member counts for LISTGetChannelCreatedAt()— channel creation time for RPL_CREATIONTIMEGetSessionCreatedAt()— session creation timeOther Changes
StartTimetoGlobalsstruct for RPL_CREATEDdocker build .passes (lint, tests, build all green).closes #52
let's send replies as
"code": 2, "command": "RPL_YOURHOST"(for example).this probably means we'll need a package simply for enumerating all of the irc numeric codes
Review: PASS ✅
Reviewed PR #59 (IRC numerics batch 2, closing #52).
Checklist
deliverWelcome(): 001→002→003→004→005→LUSERS. Correct order per RFC 2812.deliverLusers()sends 251/252/254/255, called both on connect (viadeliverWelcome) and via explicit LUSERS command.toorbody.?placeholders), no SQL injection risk.GetChannelCount(),ListAllChannelsWithCounts()(LEFT JOIN with GROUP BY),GetChannelCreatedAt(),GetSessionCreatedAt()are all clean..golangci.yml,Makefile,Dockerfile, or CI workflows. The only test change is addingStartTime: time.Now()tonewTestGlobals(), which is required by the new struct field. No assertions weakened.Minor Observation (non-blocking)
GetSessionCreatedAt()is defined inqueries.gobut not called anywhere in this PR. Likely intended for future use (WHOIS idle time, etc.). Not a problem — Go does not flag unused exported methods.Verdict
Clean, well-structured implementation that follows existing code patterns. Error handling is consistent (graceful degradation with logging in LUSERS, proper IRC error numerics for bad input). The
dispatchQueryCommandfactoring keeps the main dispatch switch manageable. RPL_ISUPPORT CHANMODES categorization is correct for the current feature set.Marking
merge-readyand assigning @sneak.i left some rework comments. also i never want bare irc numeric codes appearing in our calls, they should always be named constants from the irc numeric codes package you'll need to create.
@clawbot
bc8df117abtoa193831ed4CI Fix
The
goconstlinter flagged repeated IRC command string literals (LIST,WHO,WHOIS,QUIT,JOIN,NICK,PART,TOPIC) ininternal/handlers/api.go.Fix: Extracted all repeated command strings (≥3 occurrences) into named constants (
cmdList,cmdWho,cmdWhois,cmdQuit,cmdJoin,cmdNick,cmdPart,cmdTopic) in the existingconstblock, and replaced all inline usages.Rebased onto
main.docker build .passes (lint, tests, build all green).PR #59 Review: feature/irc-numerics-batch2
Summary
This PR adds IRC numeric reply support for MODE (query), NAMES, LIST, WHOIS, WHO, and LUSERS commands, plus connection registration numerics (001-005) and LUSERS on connect. README documentation is comprehensive and accurate.
Build Status
✅
docker build .passes (lint, fmt-check, test, build all green)Issues Found
1. Dead code: duplicate handlers (medium)
LIST,WHO, andWHOISare matched indispatchCommandat line 792:Since the explicit
casematches first, thedefaultbranch never fires for LIST/WHO/WHOIS. This means:handleList()(line 2041) is dead code —handleListCmd()(line 1610) runs insteadhandleWho()(line 2213) is dead code —handleWhoCmd()(line 1658) runs insteadhandleWhois()(line 2091) is dead code —handleWhoisCmd()(line 1720) runs insteadThe new handlers in
dispatchQueryCommandare unreachable. Either:dispatchInfoCommandand update thecaseindispatchCommand, ORdispatchQueryCommand2. Unused DB method:
GetSessionCreatedAt(low)GetSessionCreatedAtinqueries.go(line 1057) is defined but never called anywhere. Either use it or remove it.3. Minor:
PINGnot extracted to constant (nit)Other command strings (
JOIN,PART, etc.) were extracted to constants, but"PING","MOTD","MODE","NAMES", and"LUSERS"remain as string literals in various places. Not blocking, but inconsistent.What Looks Good
Globals.StartTimeis a clean addition for RPL_CREATED (003)StartTimeListAllChannelsWithCountsuses proper LEFT JOIN for accurate countsVerdict
Needs rework — the dead code / duplicate handlers issue should be resolved before merge. The dispatch logic has two parallel paths for the same commands, which will cause confusion for future contributors.
it appears you are being inconsistent in the switch statement - some are using constants, some are string literals.
@clawbot this needs rework, please do it
@clawbot this requires rework
Rework agent is already in flight — fixing all inconsistent command string literals to use named constants. Should be done shortly.
Rework: Consistent Command Constants
Fixed the inconsistency where some IRC command strings used named constants while others were bare string literals.
Constants Added
internal/handlers/api.go— added to existing const block:cmdLusers = "LUSERS"cmdMode = "MODE"cmdMotd = "MOTD"cmdNames = "NAMES"cmdNotice = "NOTICE"cmdPing = "PING"cmdPong = "PONG"cmd/neoirc-cli/main.go— new const block:cmdJoin,cmdMotd,cmdNick,cmdNotice,cmdPart,cmdPrivmsg,cmdQuit,cmdTopic,cmdWho,cmdWhoiscmd/neoirc-cli/api/client.go— new const block:cmdJoin,cmdPartReplacements Made
internal/handlers/api.go(11 replacements):"NOTICE"→cmdNotice(dispatchCommand switch)"MOTD"→cmdMotd(dispatchCommand + dispatchInfoCommand switches)"PING"→cmdPing(dispatchCommand + dispatchInfoCommand switches)"PONG"→cmdPong(PING response map)"MODE"→cmdMode(dispatchQueryCommand switch + error params)"NAMES"→cmdNames(dispatchQueryCommand switch + error params)"LUSERS"→cmdLusers(dispatchQueryCommand switch)cmd/neoirc-cli/main.go(17 replacements):Command: "PRIVMSG"→Command: cmdPrivmsg(2×)Command: "NICK"→Command: cmdNickCommand: "TOPIC"→Command: cmdTopic(2×)Command: "MOTD"→Command: cmdMotdCommand: "WHO"→Command: cmdWhoCommand: "WHOIS"→Command: cmdWhoisCommand: "QUIT"→Command: cmdQuitcmd/neoirc-cli/api/client.go(2 replacements):Command: "JOIN"→Command: cmdJoinCommand: "PART"→Command: cmdPartVerification
docker build .passes (lint, fmt-check, tests, build all green)mainReview: FAIL ❌
Reviewed PR #59 (IRC numerics batch 2), specifically the rework cycle addressing constants consistency (comment).
What Works
StartTimefield).golangci.ymlunchangeddocker build .passesBlocking Issues
1. No IRC numerics package — sneak explicitly requested a dedicated package for IRC numeric code constants (comment: "this probably means we'll need a package simply for enumerating all of the irc numeric codes") and (comment: "i never want bare irc numeric codes appearing in our calls, they should always be named constants from the irc numeric codes package you'll need to create"). No
internal/irc/package exists. There are 69 bare numeric string literals ("001","002","251","403","461", etc.) scattered throughoutinternal/handlers/api.go. These should all be named constants likeirc.RPL_WELCOME,irc.RPL_YOURHOST,irc.ERR_NOSUCHCHANNEL, etc.2. Reply format missing
"code"and named"command"fields — sneak requested (comment) that numeric replies include"code": 2, "command": "RPL_YOURHOST". Currently, numeric replies store the bare string (e.g.,"002") as thecommandfield. There is no integercodefield and no human-readable name likeRPL_YOURHOST. TheIRCMessagestruct ininternal/db/queries.goneeds aCode intJSON field, andenqueueNumericneeds to store the named constant ascommandand the integer ascode.3. Dead code: duplicate command handlers —
dispatchCommandroutescmdList,cmdWho,cmdWhoistodispatchInfoCommand(via thecase cmdMotd, cmdList, cmdWho, cmdWhois, cmdPing:branch). ButdispatchQueryCommand(called fromdefault:) also has cases for these same commands calling new implementations (handleList,handleWhois,handleWho). These new handlers are unreachable dead code — the switch indispatchCommandcatches these commands before they fall todefault. The old handlers (handleListCmd,handleWhoCmd,handleWhoisCmd) are the ones actually executing.Required Actions
Create
internal/irc/numerics.go(or similar) with named constants for ALL IRC numeric codes used in the codebase (e.g.,const RPL_WELCOME = "001",const RPL_YOURHOST = "002",const ERR_NOSUCHCHANNEL = "403", etc.). Replace all 69 bare numeric string literals inapi.gowith these constants.Update the reply format so numeric replies include both
"code": <int>and"command": "<RPL_NAME>"fields, per sneak's request. This requires updatingIRCMessage,enqueueNumeric, andInsertMessage/PollMessagesin the DB layer.Remove the dead code: either consolidate the duplicate handlers (keep the newer, improved versions in
dispatchQueryCommandand remove the old ones fromdispatchInfoCommand, adjusting the routing indispatchCommand), or remove the unreachable code fromdispatchQueryCommand.also, the duplication of constants between the cli and server need to be addressed
Rework: IRC Numerics Package + Reply Format + Dead Code Fix + Constant Deduplication
All FOUR issues addressed in a single commit.
1. New
internal/irc/Package (BLOCKING #1 + sneak's dedup request)internal/irc/numerics.go— complete IRC numeric reply code constants from RFC 1459 and RFC 2812:RplWelcome(1) throughRplIsupport(5)RplUmodeIs(221),RplLuserClient(251)–RplLuserMe(255),RplWhoisUser(311)–RplWhoisChannels(319),RplList(322)–RplCreationTime(329),RplNoTopic(331)–RplTopic(332),RplWhoReply(352),RplNamReply(353),RplEndOfNames(366),RplMotd(372)–RplEndOfMotd(376), and moreErrNoSuchNick(401)–ErrChanOpPrivsNeeded(482)Name(code int) stringfunction returns the standard IRC name (e.g.,Name(2)→"RPL_YOURHOST")internal/irc/commands.go— single source of truth for all IRC command string constants:CmdJoin,CmdList,CmdLusers,CmdMode,CmdMotd,CmdNames,CmdNick,CmdNotice,CmdPart,CmdPing,CmdPong,CmdPrivmsg,CmdQuit,CmdTopic,CmdWho,CmdWhois2. Reply Format with
codeand Namedcommand(BLOCKING #2)internal/db/queries.go:Code intfield (json:"code,omitempty") toIRCMessagestructscanMessages()now detects 3-digit numeric commands stored in the DB, parses them toint, setsCode, and replacesCommandwith the human-readable name viairc.Name(){"code": 2, "command": "RPL_YOURHOST", ...}instead of{"command": "002", ...}3. Dead Code Removal (BLOCKING #3)
Removed 4 unreachable handler methods:
handleListCmd— replaced byhandleList(usesListAllChannelsWithCountssingle-query vs N+1)handleWhoCmd— replaced byhandleWhohandleWhoisCmd— replaced byhandleWhoissendWhoisNumerics— only called by removedhandleWhoisCmdFixed routing: removed
cmdList,cmdWho,cmdWhoisfrom thecaseindispatchCommandso they now route throughdispatchQueryCommand(which has the improved implementations).dispatchInfoCommandnow only handlesMOTDandPING.4. Constant Deduplication (sneak's additional request)
Removed duplicate command constant definitions from:
internal/handlers/api.go— 16cmd*constants removed, now importsirc.Cmd*cmd/neoirc-cli/main.go— 10cmd*constants removed, now importsirc.Cmd*cmd/neoirc-cli/api/client.go— 2cmd*constants removed, now importsirc.Cmd*All three files now import from
internal/irc/— single source of truth.5. Numeric Literals Replaced
internal/handlers/api.go:enqueueNumericsignature changed fromcommand stringtocode intrespondIRCErrorsignature changed fromnumeric stringtocode int"001","002","251","403","461", etc.) replaced with named constants (irc.RplWelcome,irc.RplYourHost,irc.RplLuserClient,irc.ErrNoSuchChannel,irc.ErrNeedMoreParams, etc.)Verification
docker build .passes (lint, fmt-check, tests, build all green)mainReview: PASS ✅
Reviewed PR #59 (IRC numerics batch 2, closing issue #52). This is the final review after the rework cycle that added the
internal/irc/numerics package, fixed dead code, and deduplicated constants.Mandatory Checklist — All Items Verified
internal/irc/numerics.goexists with 60 named integer constants for IRC numeric codes (001-005, 221, 251-255, 301-319, 322-329, 331-333, 341, 352-353, 366-368, 372-376, 401-482)irc.Name(code int) stringfunction exists — returns human-readable names (e.g.,Name(2)→"RPL_YOURHOST")internal/irc/commands.goexists with 16 named command string constants (CmdJoin,CmdList,CmdLusers,CmdMode,CmdMotd,CmdNames,CmdNick,CmdNotice,CmdPart,CmdPing,CmdPong,CmdPrivmsg,CmdQuit,CmdTopic,CmdWho,CmdWhois)"code"field AND human-readable"command"field —scanMessages()inqueries.godetects 3-digit numeric commands, setsCode(int), and replacesCommandwith the named constant viairc.Name()(e.g.,{"code": 2, "command": "RPL_YOURHOST"})grepforcase "in handler and CLI code shows only/connect,/nick, etc. (user-facing slash commands), not IRC protocol stringsgrepfor"001","002","322","401", etc. in api.go, main.go, client.go returns no matches. All useirc.Rpl*/irc.Err*constantsconstdeclarations for IRC command strings ininternal/handlers/,cmd/neoirc-cli/main.go, orcmd/neoirc-cli/api/client.go. All import frominternal/irc/handleListCmd,handleWhoCmd,handleWhoisCmd,sendWhoisNumerics) removed.dispatchInfoCommandnow only handles MOTD and PING.dispatchQueryCommandhandles MODE, NAMES, LIST, WHOIS, WHO, LUSERSdeliverWelcome()sends RPL_WELCOME through RPL_ISUPPORT in correct RFC 2812 order, followed by LUSERSdeliverLusers()sends RPL_LUSERCLIENT/LUSEROP/LUSERCHANNELS/LUSERME; called on connect and via explicit LUSERS commandListAllChannelsWithCounts()(single JOIN query, no N+1), sends 322 per channel + 323GetChannelCount,ListAllChannelsWithCounts,GetChannelCreatedAt,GetSessionCreatedAt) use?placeholders. No string interpolation in SQL.golangci.yml,Makefile,Dockerfileunchanged. Test changes: onlyStartTimefield added tonewTestGlobals()(required by struct change) andfindNumeric()adapted to checkcodeinteger field (functionally equivalent to prior assertion)docker build .passes — lint, fmt-check, tests, build all greenArchitecture Notes
The
enqueueNumericandrespondIRCErrorsignatures changed fromstringtointfor the numeric code parameter, propagating type safety throughout. The conversion tofmt.Sprintf("%03d", code)happens at the DB storage boundary, and reconversion back to the structured{code: int, command: string}format happens inscanMessages(). This is clean and correct.Net change is +307/-361 — a code reduction despite adding significant functionality, achieved through dead code removal and constant deduplication.
Marking
merge-readyand assigning @sneak.