IRC commands (PRIVMSG, JOIN, PART, NICK, TOPIC, etc.) now respond with
proper IRC numeric replies delivered through the message queue instead of
HTTP status codes. HTTP error codes are now reserved exclusively for
transport-level concerns: auth failures (401), malformed requests (400),
and server errors (500).
Changes:
- Add params column to messages table for IRC-style parameters
- Add Params field to IRCMessage struct and update all queries
- Add respondIRCError helper for consistent IRC error delivery
- Add RPL_WELCOME (001) on session creation and login
- Add RPL_TOPIC/RPL_NOTOPIC (332/331), RPL_NAMREPLY (353),
RPL_ENDOFNAMES (366) on JOIN
- Add RPL_TOPIC (332) on TOPIC set
- Replace HTTP 404 with ERR_NOSUCHCHANNEL (403) and ERR_NOSUCHNICK (401)
- Replace HTTP 409 with ERR_NICKNAMEINUSE (433)
- Replace HTTP 403 with ERR_NOTONCHANNEL (442)
- Replace HTTP 400 with ERR_NEEDMOREPARAMS (461), ERR_ERRONEUSNICKNAME (432),
and ERR_UNKNOWNCOMMAND (421) where appropriate
- Change PRIVMSG/NOTICE success from HTTP 201 to HTTP 200
- Update all tests to verify IRC numerics in message queue
- Add new tests for RPL_WELCOME and JOIN numerics
- Update README to document new numeric reply behavior
closes#54
Database migration handles the new params column correctly
Tests not weakened — tests are actually strengthened (now verify HTTP 200 + poll queue for correct IRC numeric)
No linter/CI config changes
README updated to reflect new protocol behavior
docker build . passes
Implementation Quality
Architecture: Clean separation between HTTP transport errors and IRC protocol errors. The respondIRCError helper centralizes the pattern of enqueue-numeric → notify-broker → respond-HTTP-200. All IRC command errors are now delivered via the message queue as proper numeric replies, exactly as issue #54 requested.
MOTD (375/372/376) was already implemented and now correctly follows RPL_WELCOME
Tests: Two new tests added (TestWelcomeNumeric, TestJoinNumerics). All existing error tests updated from checking HTTP 4xx status codes to checking HTTP 200 + polling the message queue for the correct IRC numeric. This is more thorough testing, not less.
PRIVMSG/NOTICE: HTTP response changed from 201 Created to 200 OK, which is correct — all IRC commands now uniformly return 200.
Database:params column added to messages table (schema 001_initial.sql), InsertMessage signature updated, PollMessages and queryHistory queries updated to select/scan the new column. Pre-1.0 migration approach (modify existing migration) follows repo policy.
Minor Observations (non-blocking)
deliverJoinNumerics calls GetChannelByName redundantly (channel ID is already passed as parameter), then uses ListChannels to find the topic — slightly inefficient but correct.
The respondIRCError helper is used in some places while the same pattern is inlined in others (e.g., handlePrivmsg missing target/body, unknown command, handleDirectMsg user not found). Could be cleaned up for consistency in a follow-up.
Branch is already up to date with main. No rebase needed.
## Code Review — PR #56: Replace HTTP error codes with IRC numeric replies
**Verdict: ✅ PASS**
### Checklist
- [x] IRC commands now return IRC numerics instead of HTTP codes
- [x] HTTP codes ONLY used for session/connection/server errors (400 malformed JSON, 401 auth, 500 server)
- [x] RPL_WELCOME (001) sent on session create, register, and login
- [x] JOIN sends RPL_TOPIC/RPL_NOTOPIC (332/331) + RPL_NAMREPLY (353) + RPL_ENDOFNAMES (366)
- [x] Error cases use proper ERR_* numerics (401, 403, 421, 432, 433, 442, 461)
- [x] `enqueueNumeric()` pattern used consistently
- [x] Database migration handles the new `params` column correctly
- [x] Tests not weakened — tests are actually **strengthened** (now verify HTTP 200 + poll queue for correct IRC numeric)
- [x] No linter/CI config changes
- [x] README updated to reflect new protocol behavior
- [x] `docker build .` passes
### Implementation Quality
**Architecture:** Clean separation between HTTP transport errors and IRC protocol errors. The `respondIRCError` helper centralizes the pattern of enqueue-numeric → notify-broker → respond-HTTP-200. All IRC command errors are now delivered via the message queue as proper numeric replies, exactly as [issue #54](https://git.eeqj.de/sneak/chat/issues/54) requested.
**Numerics implemented:**
- Success: 001 (RPL_WELCOME), 331 (RPL_NOTOPIC), 332 (RPL_TOPIC), 353 (RPL_NAMREPLY), 366 (RPL_ENDOFNAMES)
- Errors: 401 (ERR_NOSUCHNICK), 403 (ERR_NOSUCHCHANNEL), 421 (ERR_UNKNOWNCOMMAND), 432 (ERR_ERRONEUSNICKNAME), 433 (ERR_NICKNAMEINUSE), 442 (ERR_NOTONCHANNEL), 461 (ERR_NEEDMOREPARAMS)
- MOTD (375/372/376) was already implemented and now correctly follows RPL_WELCOME
**Tests:** Two new tests added (`TestWelcomeNumeric`, `TestJoinNumerics`). All existing error tests updated from checking HTTP 4xx status codes to checking HTTP 200 + polling the message queue for the correct IRC numeric. This is more thorough testing, not less.
**PRIVMSG/NOTICE:** HTTP response changed from 201 Created to 200 OK, which is correct — all IRC commands now uniformly return 200.
**Database:** `params` column added to messages table (schema `001_initial.sql`), `InsertMessage` signature updated, `PollMessages` and `queryHistory` queries updated to select/scan the new column. Pre-1.0 migration approach (modify existing migration) follows repo policy.
### Minor Observations (non-blocking)
1. `deliverJoinNumerics` calls `GetChannelByName` redundantly (channel ID is already passed as parameter), then uses `ListChannels` to find the topic — slightly inefficient but correct.
2. The `respondIRCError` helper is used in some places while the same pattern is inlined in others (e.g., `handlePrivmsg` missing target/body, unknown command, handleDirectMsg user not found). Could be cleaned up for consistency in a follow-up.
Branch is already up to date with `main`. No rebase needed.
<!-- session: agent:sdlc-manager:subagent:b986a0d5-a037-463e-bb2e-a0fe9bf7e441 -->
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Refactors all IRC command handlers to respond with proper IRC numeric replies via the message queue instead of HTTP status codes.
HTTP error codes are now reserved exclusively for transport-level concerns:
IRC Numerics Implemented
Success replies (delivered via message queue on success):
Error replies (delivered via message queue instead of HTTP 4xx):
Database Changes
paramscolumn to messages table for IRC-style parametersParamsfield toIRCMessagestructInsertMessageto accept paramsTest Updates
TestWelcomeNumeric,TestJoinNumericsClient Impact
closes sneak/chat#54
Code Review — PR #56: Replace HTTP error codes with IRC numeric replies
Verdict: ✅ PASS
Checklist
enqueueNumeric()pattern used consistentlyparamscolumn correctlydocker build .passesImplementation Quality
Architecture: Clean separation between HTTP transport errors and IRC protocol errors. The
respondIRCErrorhelper centralizes the pattern of enqueue-numeric → notify-broker → respond-HTTP-200. All IRC command errors are now delivered via the message queue as proper numeric replies, exactly as issue #54 requested.Numerics implemented:
Tests: Two new tests added (
TestWelcomeNumeric,TestJoinNumerics). All existing error tests updated from checking HTTP 4xx status codes to checking HTTP 200 + polling the message queue for the correct IRC numeric. This is more thorough testing, not less.PRIVMSG/NOTICE: HTTP response changed from 201 Created to 200 OK, which is correct — all IRC commands now uniformly return 200.
Database:
paramscolumn added to messages table (schema001_initial.sql),InsertMessagesignature updated,PollMessagesandqueryHistoryqueries updated to select/scan the new column. Pre-1.0 migration approach (modify existing migration) follows repo policy.Minor Observations (non-blocking)
deliverJoinNumericscallsGetChannelByNameredundantly (channel ID is already passed as parameter), then usesListChannelsto find the topic — slightly inefficient but correct.respondIRCErrorhelper is used in some places while the same pattern is inlined in others (e.g.,handlePrivmsgmissing target/body, unknown command, handleDirectMsg user not found). Could be cleaned up for consistency in a follow-up.Branch is already up to date with
main. No rebase needed.