sneak/chat
1
0
Fork 0
Code Issues 7 Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: replace HTTP error codes with IRC numeric replies #56

Merged
sneak merged 2 commits from feature/irc-numeric-replies into main 2026-03-09 22:21:31 +01:00
Conversation 1 Commits 2 Files Changed 7 +627 -174
clawbot commented 2026-03-08 10:32:25 +01:00
Collaborator
Copy Link

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:

  • 401 — missing/invalid auth token
  • 400 — malformed JSON, empty command
  • 500 — server errors

IRC Numerics Implemented

Success replies (delivered via message queue on success):

  • 001 RPL_WELCOME — sent on session creation and login
  • 331 RPL_NOTOPIC — channel has no topic (on JOIN)
  • 332 RPL_TOPIC — channel topic (on JOIN, TOPIC set)
  • 353 RPL_NAMREPLY — channel member list (on JOIN)
  • 366 RPL_ENDOFNAMES — end of NAMES list (on JOIN)
  • 375/372/376 — MOTD (already existed)

Error replies (delivered via message queue instead of HTTP 4xx):

  • 401 ERR_NOSUCHNICK — DM target not found (was HTTP 404)
  • 403 ERR_NOSUCHCHANNEL — channel not found / invalid name (was HTTP 404)
  • 421 ERR_UNKNOWNCOMMAND — unrecognized command (was HTTP 400)
  • 432 ERR_ERRONEUSNICKNAME — invalid nick format (was HTTP 400)
  • 433 ERR_NICKNAMEINUSE — nick taken (was HTTP 409)
  • 442 ERR_NOTONCHANNEL — not a member of channel (was HTTP 403)
  • 461 ERR_NEEDMOREPARAMS — missing required fields (was HTTP 400)

Database Changes

  • Added params column to messages table for IRC-style parameters
  • Added Params field to IRCMessage struct
  • Updated InsertMessage to accept params

Test Updates

  • All existing tests updated to expect HTTP 200 + IRC numerics
  • New tests: TestWelcomeNumeric, TestJoinNumerics

Client Impact

  • CLI and SPA already handle unknown numerics via default event handlers
  • PRIVMSG/NOTICE success changed from HTTP 201 to HTTP 200

closes #54

## 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: - **401** — missing/invalid auth token - **400** — malformed JSON, empty command - **500** — server errors ## IRC Numerics Implemented ### Success replies (delivered via message queue on success): - **001 RPL_WELCOME** — sent on session creation and login - **331 RPL_NOTOPIC** — channel has no topic (on JOIN) - **332 RPL_TOPIC** — channel topic (on JOIN, TOPIC set) - **353 RPL_NAMREPLY** — channel member list (on JOIN) - **366 RPL_ENDOFNAMES** — end of NAMES list (on JOIN) - **375/372/376** — MOTD (already existed) ### Error replies (delivered via message queue instead of HTTP 4xx): - **401 ERR_NOSUCHNICK** — DM target not found (was HTTP 404) - **403 ERR_NOSUCHCHANNEL** — channel not found / invalid name (was HTTP 404) - **421 ERR_UNKNOWNCOMMAND** — unrecognized command (was HTTP 400) - **432 ERR_ERRONEUSNICKNAME** — invalid nick format (was HTTP 400) - **433 ERR_NICKNAMEINUSE** — nick taken (was HTTP 409) - **442 ERR_NOTONCHANNEL** — not a member of channel (was HTTP 403) - **461 ERR_NEEDMOREPARAMS** — missing required fields (was HTTP 400) ## Database Changes - Added `params` column to messages table for IRC-style parameters - Added `Params` field to `IRCMessage` struct - Updated `InsertMessage` to accept params ## Test Updates - All existing tests updated to expect HTTP 200 + IRC numerics - New tests: `TestWelcomeNumeric`, `TestJoinNumerics` ## Client Impact - CLI and SPA already handle unknown numerics via default event handlers - PRIVMSG/NOTICE success changed from HTTP 201 to HTTP 200 closes https://git.eeqj.de/sneak/chat/issues/54
clawbot added 1 commit 2026-03-08 10:32:25 +01:00
refactor: replace HTTP error codes with IRC numeric replies for all IRC commands
All checks were successful
check / check (push) Successful in 58s
Details
f9c145ad09 
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
clawbot added the botneeds-review labels 2026-03-08 10:32:40 +01:00
clawbot removed the needs-review label 2026-03-08 10:33:50 +01:00
clawbot commented 2026-03-08 10:36:45 +01:00
Author
Collaborator
Copy Link

Code Review — PR #56: Replace HTTP error codes with IRC numeric replies

Verdict: ✅ PASS

Checklist

  • IRC commands now return IRC numerics instead of HTTP codes
  • HTTP codes ONLY used for session/connection/server errors (400 malformed JSON, 401 auth, 500 server)
  • RPL_WELCOME (001) sent on session create, register, and login
  • JOIN sends RPL_TOPIC/RPL_NOTOPIC (332/331) + RPL_NAMREPLY (353) + RPL_ENDOFNAMES (366)
  • Error cases use proper ERR_* numerics (401, 403, 421, 432, 433, 442, 461)
  • enqueueNumeric() pattern used consistently
  • 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.

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.

## 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 -->
clawbot added merge-ready and removed bot labels 2026-03-08 10:37:04 +01:00
sneak was assigned by clawbot 2026-03-08 10:37:13 +01:00
sneak added 1 commit 2026-03-09 22:17:55 +01:00
Merge branch 'main' into feature/irc-numeric-replies
All checks were successful
check / check (push) Successful in 2m17s
Details
8157b6d280
sneak merged commit f8f0b6afbb into main 2026-03-09 22:21:31 +01:00
sneak deleted branch feature/irc-numeric-replies 2026-03-09 22:21:31 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bot
Agent's turn to work
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/chat#56
Delete Branch "feature/irc-numeric-replies"

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?