Complete IRC numerics module and move to pkg/irc/ (refs #52) #71

Merged
sneak merged 5 commits from feat/complete-irc-numerics into main 2026-03-10 18:41:27 +01:00
Collaborator

This PR addresses issue #52:

  • Adds all missing numeric reply codes from RFC 1459 and RFC 2812, making the module spec-complete
  • Moves the package from internal/irc/ to pkg/irc/ to indicate external usefulness
  • Updates all imports throughout the codebase

Added numerics

  • Trace replies (200-209)
  • Stats replies (211-219, 242-243)
  • Service replies (234-235)
  • Admin replies (256-259)
  • Trace log/end, try again (261-263)
  • WHOWAS (314, 369)
  • List start (321), unique ops (325)
  • Invite/except lists (346-349)
  • Version (351), links (364-365)
  • Info (371, 374)
  • Oper/rehash/service (381-383)
  • Time/users (391-395)
  • All missing error codes (406-415, 422-424, 436-437, 443-446, 463-467, 472, 476-478, 481, 483-485, 491, 501-502)

refs #52

This PR addresses [issue #52](https://git.eeqj.de/sneak/chat/issues/52): - **Adds all missing numeric reply codes** from RFC 1459 and RFC 2812, making the module spec-complete - **Moves the package** from `internal/irc/` to `pkg/irc/` to indicate external usefulness - **Updates all imports** throughout the codebase ### Added numerics - Trace replies (200-209) - Stats replies (211-219, 242-243) - Service replies (234-235) - Admin replies (256-259) - Trace log/end, try again (261-263) - WHOWAS (314, 369) - List start (321), unique ops (325) - Invite/except lists (346-349) - Version (351), links (364-365) - Info (371, 374) - Oper/rehash/service (381-383) - Time/users (391-395) - All missing error codes (406-415, 422-424, 436-437, 443-446, 463-467, 472, 476-478, 481, 483-485, 491, 501-502) refs #52
clawbot added 1 commit 2026-03-10 18:07:09 +01:00
Complete IRC numerics module and move to pkg/irc/ (refs #52)
All checks were successful
check / check (push) Successful in 2m18s
53a824c01b
- Add all missing numeric reply codes from RFC 1459 and RFC 2812
- Move irc package from internal/irc/ to pkg/irc/ for external use
- Update all imports throughout the codebase
- Added: trace replies (200-209), stats replies (211-219, 242-243),
  admin replies (256-259), service replies (234-235), WHOWAS (314, 369),
  list start (321), version (351), links (364-365), info (371, 374),
  oper/rehash/service (381-383), time/users (391-395),
  invite/except lists (346-349), unique ops (325),
  and all missing error codes (406-415, 422-424, 436-437, 443-446,
  463-467, 472, 476-478, 481, 483-485, 491, 501-502)
clawbot added the needs-review label 2026-03-10 18:07:31 +01:00
Author
Collaborator

Review: FAIL — Missing RFC 2812 Numerics

What works

  • Package move from internal/irc/ to pkg/irc/ is clean
  • All 4 import sites updated correctly, no stale internal/irc references remain
  • commands.go moved via rename (no content changes)
  • No cheating: Makefile, linter config, CI, and test assertions are untouched
  • docker build . passes
  • The names map is consistent with the constants
  • Good handling of the RplBounce/RplIsupport dual-constant for numeric 5

Missing RFC 2812 numerics

The PR claims to be spec-complete for RFC 1459 and RFC 2812, but several numerics from RFC 2812 section 5 are absent:

Code Name RFC 2812 Section
217 RPL_STATSQLINE 5.1
241 RPL_STATSLLINE 5.1
244 RPL_STATSHLINE 5.1

These are explicitly defined in RFC 2812 and should be present for spec-completeness.

Please add these constants and their entries in the names map.

Result: FAIL — needs rework to add the missing numerics above.

Reviewed PR #71 for issue #52

## Review: FAIL — Missing RFC 2812 Numerics ### What works - Package move from `internal/irc/` to `pkg/irc/` is clean - All 4 import sites updated correctly, no stale `internal/irc` references remain - `commands.go` moved via rename (no content changes) - No cheating: Makefile, linter config, CI, and test assertions are untouched - `docker build .` passes - The names map is consistent with the constants - Good handling of the RplBounce/RplIsupport dual-constant for numeric 5 ### Missing RFC 2812 numerics The PR claims to be spec-complete for RFC 1459 and RFC 2812, but several numerics from RFC 2812 section 5 are absent: | Code | Name | RFC 2812 Section | |------|------|------------------| | 217 | RPL_STATSQLINE | 5.1 | | 241 | RPL_STATSLLINE | 5.1 | | 244 | RPL_STATSHLINE | 5.1 | These are explicitly defined in RFC 2812 and should be present for spec-completeness. Please add these constants and their entries in the names map. **Result: FAIL** — needs rework to add the missing numerics above. *Reviewed [PR #71](https://git.eeqj.de/sneak/chat/pulls/71) for [issue #52](https://git.eeqj.de/sneak/chat/issues/52)*
clawbot added needs-rework and removed needs-review labels 2026-03-10 18:10:37 +01:00
clawbot added 1 commit 2026-03-10 18:12:49 +01:00
Author
Collaborator

Rework complete. Added the 3 missing RFC 2812 numerics:

  • RPL_STATSQLINE (217)
  • RPL_STATSLLINE (241)
  • RPL_STATSHLINE (244)

All three added as constants and to the Names map. docker build . passes.

Rework complete. Added the 3 missing RFC 2812 numerics: - `RPL_STATSQLINE` (217) - `RPL_STATSLLINE` (241) - `RPL_STATSHLINE` (244) All three added as constants and to the `Names` map. `docker build .` passes.
clawbot added needs-review and removed needs-rework labels 2026-03-10 18:13:05 +01:00
Owner

Make a custom int type called IRCMessageType and define a .Name() on it and update all places in the code that use these numeric codes to take an IRCMessageType

Make a custom int type called IRCMessageType and define a .Name() on it and update all places in the code that use these numeric codes to take an IRCMessageType
clawbot added needs-rework and removed needs-review labels 2026-03-10 18:13:39 +01:00
clawbot self-assigned this 2026-03-10 18:13:40 +01:00
Author
Collaborator

On it — creating IRCMessageType custom int type with .Name() method and updating all usages across the codebase.

On it — creating `IRCMessageType` custom int type with `.Name()` method and updating all usages across the codebase.
Author
Collaborator

Review: PASS

All three previously missing RFC 2812 numerics are now present:

  • RplStatsQLine = 217
  • RplStatsLLine = 241
  • RplStatsHLine = 244

The module now covers all RFC 1459 and RFC 2812 numeric reply codes comprehensively (001-005, 200-395, 401-502), with both constants and the names lookup map entries for each.

All four import sites updated from internal/irc to pkg/irc. commands.go correctly moved alongside numerics.go.

docker build . (lint + tests + build) passes clean.

Refs PR #71, issue #52.

## Review: PASS All three previously missing RFC 2812 numerics are now present: - `RplStatsQLine = 217` - `RplStatsLLine = 241` - `RplStatsHLine = 244` The module now covers all RFC 1459 and RFC 2812 numeric reply codes comprehensively (001-005, 200-395, 401-502), with both constants and the `names` lookup map entries for each. All four import sites updated from `internal/irc` to `pkg/irc`. `commands.go` correctly moved alongside `numerics.go`. `docker build .` (lint + tests + build) passes clean. Refs [PR #71](https://git.eeqj.de/sneak/chat/pulls/71), [issue #52](https://git.eeqj.de/sneak/chat/issues/52).
clawbot added 1 commit 2026-03-10 18:17:59 +01:00
refactor: add irc.MessageType custom type with Name() and String() methods
All checks were successful
check / check (push) Successful in 2m28s
164fec3fc8
- Create 'type MessageType int' in pkg/irc/numerics.go
- Type all Rpl*/Err* numeric constants as MessageType
- Add Name() method returning the standard IRC name (e.g. RPL_WELCOME)
- Add String() method returning zero-padded numeric (e.g. 001)
- Update enqueueNumeric/respondIRCError to accept MessageType
- Update internal/db to use MessageType conversion for Name() calls
- Change names map key type from int to MessageType

Refs #52
Owner

.String() should be like RPL_LUSEROP <252> and .Code() should give the zero-padded string and .Int() should give the bare int and .Name() should give RPL_LUSEROP. There should also be an irc.FromInt() that returns (error, IRCMessageType).

.String() should be like `RPL_LUSEROP <252>` and .Code() should give the zero-padded string and .Int() should give the bare int and .Name() should give `RPL_LUSEROP`. There should also be an `irc.FromInt()` that returns (error, IRCMessageType).
Author
Collaborator

Acknowledged updated spec — implementing IRCMessageType with:

  • .Name()"RPL_LUSEROP"
  • .String()"RPL_LUSEROP <252>"
  • .Code()"252" (zero-padded)
  • .Int() → bare int
  • irc.FromInt(n)(IRCMessageType, error)

Killed previous rework agent (had older spec), dispatching fresh one now.

Acknowledged updated spec — implementing `IRCMessageType` with: - `.Name()` → `"RPL_LUSEROP"` - `.String()` → `"RPL_LUSEROP <252>"` - `.Code()` → `"252"` (zero-padded) - `.Int()` → bare int - `irc.FromInt(n)` → `(IRCMessageType, error)` Killed previous rework agent (had older spec), dispatching fresh one now.
Owner

@clawbot report status

@clawbot report status
Author
Collaborator

Rework agent is running (8min in) — implementing IRCMessageType with full API (.Name(), .String(), .Code(), .Int(), FromInt()). Will push when ready.

Rework agent is running (8min in) — implementing `IRCMessageType` with full API (`.Name()`, `.String()`, `.Code()`, `.Int()`, `FromInt()`). Will push when ready.
clawbot added 1 commit 2026-03-10 18:31:24 +01:00
refactor: rename MessageType to IRCMessageType with full API
All checks were successful
check / check (push) Successful in 4s
ba2c113a85
- Rename MessageType to IRCMessageType (custom int type)
- .Name() returns just the name (e.g. "RPL_LUSEROP")
- .String() returns name + code (e.g. "RPL_LUSEROP <252>")
- .Code() returns zero-padded 3-digit string (e.g. "252")
- .Int() returns the bare int value
- Add FromInt() package-level function with validation
- Update all call sites to use new type and methods
Author
Collaborator

Reworked per feedback:

  • Renamed MessageTypeIRCMessageType (custom int type)
  • .Name() → returns just the name, e.g. "RPL_LUSEROP"
  • .String() → returns name + code, e.g. "RPL_LUSEROP <252>"
  • .Code() → returns zero-padded 3-digit string, e.g. "252"
  • .Int() → returns the bare int value
  • Added FromInt(n int) (IRCMessageType, error) — validates known numerics
  • Updated all call sites in internal/db/queries.go and internal/handlers/api.go
  • docker build . passes (lint + tests + build)

Ref: PR #71, Issue #52

Reworked per feedback: - Renamed `MessageType` → `IRCMessageType` (custom `int` type) - `.Name()` → returns just the name, e.g. `"RPL_LUSEROP"` - `.String()` → returns name + code, e.g. `"RPL_LUSEROP <252>"` - `.Code()` → returns zero-padded 3-digit string, e.g. `"252"` - `.Int()` → returns the bare int value - Added `FromInt(n int) (IRCMessageType, error)` — validates known numerics - Updated all call sites in `internal/db/queries.go` and `internal/handlers/api.go` - `docker build .` passes (lint + tests + build) Ref: [PR #71](https://git.eeqj.de/sneak/chat/pulls/71), [Issue #52](https://git.eeqj.de/sneak/chat/issues/52)
clawbot added needs-review and removed needs-rework labels 2026-03-10 18:31:54 +01:00
Author
Collaborator

Review: PASS

IRCMessageType API — verified correct

  • IRCMessageType is type IRCMessageType int
  • .Name() → bare name (e.g. "RPL_LUSEROP") ✓
  • .String()"RPL_LUSEROP <252>" format ✓
  • .Code() → zero-padded 3-digit string ✓
  • .Int() → bare int ✓
  • FromInt(n int) (IRCMessageType, error) validates known numerics ✓

Call sites — all updated

  • enqueueNumeric and respondIRCError signatures use irc.IRCMessageType
  • enqueueNumeric uses code.Code() for JSON command field (not .String()) ✓
  • All 40+ call sites use typed constants, no bare ints ✓
  • queries.go uses irc.FromInt() then .Name()
  • All imports updated to pkg/irc

Build

  • docker build . passes (lint + tests + build) ✓
  • No cheating (Makefile, linter config, test assertions untouched) ✓

All sneak feedback addressed. Clean implementation.

## Review: PASS ### IRCMessageType API — verified correct - `IRCMessageType` is `type IRCMessageType int` ✓ - `.Name()` → bare name (e.g. `"RPL_LUSEROP"`) ✓ - `.String()` → `"RPL_LUSEROP <252>"` format ✓ - `.Code()` → zero-padded 3-digit string ✓ - `.Int()` → bare int ✓ - `FromInt(n int) (IRCMessageType, error)` validates known numerics ✓ ### Call sites — all updated - `enqueueNumeric` and `respondIRCError` signatures use `irc.IRCMessageType` ✓ - `enqueueNumeric` uses `code.Code()` for JSON command field (not `.String()`) ✓ - All 40+ call sites use typed constants, no bare ints ✓ - `queries.go` uses `irc.FromInt()` then `.Name()` ✓ - All imports updated to `pkg/irc` ✓ ### Build - `docker build .` passes (lint + tests + build) ✓ - No cheating (Makefile, linter config, test assertions untouched) ✓ All sneak feedback addressed. Clean implementation.
clawbot added merge-ready and removed needs-review labels 2026-03-10 18:34:42 +01:00
clawbot removed their assignment 2026-03-10 18:34:43 +01:00
sneak was assigned by clawbot 2026-03-10 18:34:43 +01:00
Owner

write me some basic tests pls

write me some basic tests pls
clawbot added needs-rework and removed merge-ready labels 2026-03-10 18:36:09 +01:00
sneak was unassigned by clawbot 2026-03-10 18:36:10 +01:00
clawbot self-assigned this 2026-03-10 18:36:10 +01:00
Author
Collaborator

On it — adding tests for IRCMessageType API. Dispatching rework agent now.

On it — adding tests for `IRCMessageType` API. Dispatching rework agent now.
clawbot added 1 commit 2026-03-10 18:38:58 +01:00
test: add comprehensive tests for IRCMessageType API
All checks were successful
check / check (push) Successful in 4s
eeb0f29465
Cover Name(), String(), Code(), Int(), FromInt() for known and unknown
numerics, plus the deprecated Name() package-level function.
100% statement coverage on pkg/irc.
Author
Collaborator

Added pkg/irc/numerics_test.go with 9 test functions covering the full IRCMessageType API:

  • Name() — correct name for known numerics (001, 005, 252, 404, 433)
  • String() — formatted output like "RPL_LUSEROP <252>"
  • Code() — zero-padded 3-digit string
  • Int() — bare integer value
  • FromInt() — success for known numerics, ErrUnknownNumeric for unknown
  • Unknown numeric behavior — empty Name(), "UNKNOWN <999>" for String()
  • Deprecated Name() package-level function

100% statement coverage on pkg/irc. docker build . passes (lint + tests + build).

Added `pkg/irc/numerics_test.go` with 9 test functions covering the full `IRCMessageType` API: - `Name()` — correct name for known numerics (001, 005, 252, 404, 433) - `String()` — formatted output like `"RPL_LUSEROP <252>"` - `Code()` — zero-padded 3-digit string - `Int()` — bare integer value - `FromInt()` — success for known numerics, `ErrUnknownNumeric` for unknown - Unknown numeric behavior — empty `Name()`, `"UNKNOWN <999>"` for `String()` - Deprecated `Name()` package-level function **100% statement coverage** on `pkg/irc`. `docker build .` passes (lint + tests + build).
clawbot added needs-review and removed needs-rework labels 2026-03-10 18:39:39 +01:00
Author
Collaborator

Review: PASS

Tests are thorough and correct. All 9 test functions cover the full IRCMessageType API:

Test Coverage
TestName .Name() for 5 known numerics
TestString .String() format NAME <NNN> for 4 numerics
TestCode .Code() zero-padded output for 4 numerics
TestInt .Int() bare int for 4 numerics
TestFromInt_Known FromInt() roundtrip for 5 known codes
TestFromInt_Unknown FromInt() returns ErrUnknownNumeric for 0, 999, 600, -1
TestUnknownNumeric_Name Unknown returns empty string
TestUnknownNumeric_String Unknown returns UNKNOWN <999>
TestDeprecatedNameFunc Backward-compat Name() function

All sneak feedback from PR #71 addressed. docker build . passes. Good to merge.

## Review: PASS Tests are thorough and correct. All 9 test functions cover the full `IRCMessageType` API: | Test | Coverage | |------|----------| | `TestName` | `.Name()` for 5 known numerics | | `TestString` | `.String()` format `NAME <NNN>` for 4 numerics | | `TestCode` | `.Code()` zero-padded output for 4 numerics | | `TestInt` | `.Int()` bare int for 4 numerics | | `TestFromInt_Known` | `FromInt()` roundtrip for 5 known codes | | `TestFromInt_Unknown` | `FromInt()` returns `ErrUnknownNumeric` for 0, 999, 600, -1 | | `TestUnknownNumeric_Name` | Unknown returns empty string | | `TestUnknownNumeric_String` | Unknown returns `UNKNOWN <999>` | | `TestDeprecatedNameFunc` | Backward-compat `Name()` function | All sneak feedback from [PR #71](https://git.eeqj.de/sneak/chat/pulls/71) addressed. `docker build .` passes. Good to merge.
clawbot added merge-ready and removed needs-review labels 2026-03-10 18:41:08 +01:00
clawbot removed their assignment 2026-03-10 18:41:09 +01:00
sneak was assigned by clawbot 2026-03-10 18:41:09 +01:00
sneak merged commit 27df999942 into main 2026-03-10 18:41:27 +01:00
sneak deleted branch feat/complete-irc-numerics 2026-03-10 18:41:27 +01:00
Sign in to join this conversation.