Implement core IRC channel functionality:
1. Channel member flags: is_operator and is_voiced columns in
channel_members table (proper boolean columns per sneak's
instruction, not text string modes).
2. MODE +o/+v/-o/-v: Grant/revoke operator and voice status.
Permission-checked (only +o can grant). NAMES replies show
@nick for operators and +nick for voiced users.
3. MODE +m (moderated): Only +o and +v users can send PRIVMSG
or NOTICE to moderated channels. Others get ERR_CANNOTSENDTOCHAN.
4. MODE +t (topic lock): Only +o can change topic when active.
Default ON for new channels (standard IRC behavior). Others
get ERR_CHANOPRIVSNEEDED.
5. KICK command: Operator-only, removes user from channel,
broadcasts to all members including kicked user.
6. NOTICE differentiation: No RPL_AWAY auto-reply, skips hashcash
validation on +H channels per RFC 2812.
Additional improvements:
- Channel creator auto-gets +o on first JOIN
- ISUPPORT now advertises PREFIX=(ov)@+ and CHANMODES=,,H,mnst
- MODE query shows accurate +nt/+m/+H mode string
- Fixed pre-existing unparam lint issue in fanOutSilent
Includes 22 new tests covering all requirements.
closes#85
The README says "mnst" (no i), but the server sends "imnst" (with i). This is a factual documentation error — the README doesn't match the server's actual behavior.
Furthermore, this creates an internal inconsistency within the code itself:
CHANMODES in 005: advertises ,,H,mnst (does NOT include i)
The same server simultaneously claims it supports +i in one place and doesn't in another. The fix should be to update api.go:450 to "mnst" to match both the README and the CHANMODES string.
Code Quality Notes (non-blocking)
Overall, the implementation is well-structured. Specific positives:
Proper integer columns for boolean flags (per sneak's instruction) — not text string modes
Clean separation: requireChannelOp, resolveUserModeTarget, setChannelFlag are well-factored helpers
fanOutSilent cleanup (removing unused meta param) is a nice drive-by fix
KICK correctly broadcasts to the kicked user BEFORE removal from the channel
Moderation correctly blocks both PRIVMSG and NOTICE (with test TestModeratedNoticeBlocked)
KICK uses manual enqueue loop (not fanOut) to support the params field for target nick — correct approach
Minor Observations (not blocking, for awareness)
+s in CHANMODES but unenforced: +s (secret) is still advertised in CHANMODES=,,H,mnst and RPL_MYINFO, but is documented as "Not yet enforced" and has no server-side handling. A client sending MODE #channel +s would get an unknown mode error. This is pre-existing but worth noting since +i was deliberately removed for the same reason.
Missing 441 and 404 from numeric reply reference table: The new KICK feature sends 441 (ERR_USERNOTINCHANNEL) and +m enforcement sends 404 (ERR_CANNOTSENDTOCHAN), but neither appears in the numeric codes reference table (lines 1080-1118). The KICK section documents 441, but the centralized table should be comprehensive for client developers. (404 absence is pre-existing.)
Pre-existing: setHashcashMode/clearHashcashMode have no operator check: The new mode handlers all correctly use requireChannelOp, but the pre-existing +H/-H handlers at lines 2798/2862 have no op check. Any channel member can set/clear hashcash. Not introduced by this PR.
Summary
The implementation is solid — schema design, permission model, tests, and KICK broadcast logic are all correct. The single blocking issue is the RPL_MYINFO code/README mismatch at api.go:450 where "imnst" should be "mnst". One-line fix.
## Review: PR #88 — Tier 1 Channel Modes (+o/+v/+m/+t), KICK, NOTICE
**Verdict: FAIL**
### Build Status
`docker build .` passes — lint (0 issues), fmt-check, and all 22 new tests green. No build regressions.
### Requirements Checklist
- [x] `is_operator INTEGER NOT NULL DEFAULT 0` on `channel_members` — ✅ schema correct
- [x] `is_voiced INTEGER NOT NULL DEFAULT 0` on `channel_members` — ✅ schema correct
- [x] Channel creator auto-gets `+o` on first JOIN — ✅ `CountChannelMembers` → `JoinChannelAsOperator`
- [x] MODE +o/+v/-o/-v only by existing `+o` — ✅ `requireChannelOp` gate
- [x] NAMES shows `@nick` and `+nick` prefixes — ✅ `memberPrefix()` function
- [x] ISUPPORT `PREFIX=(ov)@+` — ✅ in `deliverWelcome` 005 params
- [x] `is_moderated INTEGER NOT NULL DEFAULT 0` on channels — ✅ schema correct
- [x] `+m` enforcement: only `+o`/`+v` can send, others get `ERR_CANNOTSENDTOCHAN` — ✅ `checkModeratedSend`
- [x] `is_topic_locked INTEGER NOT NULL DEFAULT 1` on channels — ✅ schema correct, default +t ON
- [x] `+t` enforcement: only `+o` can change topic — ✅ `executeTopic` checks `IsChannelTopicLocked`
- [x] KICK command with `+o` check, target removal, broadcast — ✅ full implementation with `validateKick`/`broadcastKick`
- [x] NOTICE skips AWAY replies — ✅ `command != irc.CmdNotice` gate in `handleDirectMsg`
- [x] NOTICE skips hashcash on `+H` channels — ✅ `isNotice` check before `validateChannelHashcash`
- [x] Tests for all of the above — ✅ 22 tests covering all features
- [ ] README updated: remove "not yet enforced" caveat — ❌ **Partially done but introduces inaccuracy** (see below)
### Blocking Issue: RPL_MYINFO (004) code/README mismatch
The README at line 1082 was updated to show:
```json
{"command":"004","to":"alice","params":["neoirc","0.1","","mnst"]}
```
But the actual code at [`api.go:450`](https://git.eeqj.de/sneak/chat/src/branch/feature/tier1-channel-modes-85/internal/handlers/api.go#L450) still sends:
```go
[]string{srvName, version, "", "imnst"},
```
The README says `"mnst"` (no `i`), but the server sends `"imnst"` (with `i`). This is a factual documentation error — the README doesn't match the server's actual behavior.
Furthermore, this creates an internal inconsistency within the code itself:
- **RPL_MYINFO (004)**: advertises channel modes `imnst` (includes `i`)
- **CHANMODES in 005**: advertises `,,H,mnst` (does NOT include `i`)
The same server simultaneously claims it supports `+i` in one place and doesn't in another. The fix should be to update `api.go:450` to `"mnst"` to match both the README and the CHANMODES string.
### Code Quality Notes (non-blocking)
Overall, the implementation is well-structured. Specific positives:
- Proper integer columns for boolean flags (per sneak's instruction) — not text string modes
- Clean separation: `requireChannelOp`, `resolveUserModeTarget`, `setChannelFlag` are well-factored helpers
- `fanOutSilent` cleanup (removing unused `meta` param) is a nice drive-by fix
- KICK correctly broadcasts to the kicked user BEFORE removal from the channel
- Moderation correctly blocks both PRIVMSG and NOTICE (with test `TestModeratedNoticeBlocked`)
- KICK uses manual enqueue loop (not `fanOut`) to support the `params` field for target nick — correct approach
### Minor Observations (not blocking, for awareness)
1. **`+s` in CHANMODES but unenforced**: `+s` (secret) is still advertised in `CHANMODES=,,H,mnst` and RPL_MYINFO, but is documented as "Not yet enforced" and has no server-side handling. A client sending `MODE #channel +s` would get an unknown mode error. This is pre-existing but worth noting since `+i` was deliberately removed for the same reason.
2. **Missing `441` and `404` from numeric reply reference table**: The new KICK feature sends `441` (ERR_USERNOTINCHANNEL) and +m enforcement sends `404` (ERR_CANNOTSENDTOCHAN), but neither appears in the numeric codes reference table (lines 1080-1118). The KICK section documents 441, but the centralized table should be comprehensive for client developers. (404 absence is pre-existing.)
3. **Pre-existing: `setHashcashMode`/`clearHashcashMode` have no operator check**: The new mode handlers all correctly use `requireChannelOp`, but the pre-existing +H/-H handlers at lines 2798/2862 have no op check. Any channel member can set/clear hashcash. Not introduced by this PR.
### Summary
The implementation is solid — schema design, permission model, tests, and KICK broadcast logic are all correct. The single blocking issue is the RPL_MYINFO code/README mismatch at `api.go:450` where `"imnst"` should be `"mnst"`. One-line fix.
Fixed the RPL_MYINFO (004) channel mode string at api.go:450:
Before:"imnst" (included i for invite-only, which is Tier 2 / #86)
After:"mnst" (matches CHANMODES in 005 and README)
docker build . passes — lint, fmt-check, and all tests green.
## Rework Complete
Fixed the RPL_MYINFO (004) channel mode string at [`api.go:450`](https://git.eeqj.de/sneak/chat/src/branch/feature/tier1-channel-modes-85/internal/handlers/api.go#L450):
- **Before:** `"imnst"` (included `i` for invite-only, which is Tier 2 / [#86](https://git.eeqj.de/sneak/chat/issues/86))
- **After:** `"mnst"` (matches CHANMODES in 005 and README)
`docker build .` passes — lint, fmt-check, and all tests green.
All four references are consistent. +i (invite-only) correctly excluded — it's Tier 2 (#86).
Build
docker build . passes — lint (0 issues), fmt-check, and all tests green. The rework commit (3a6fc92) is a single-line diff, no regressions.
Summary
The blocking issue from the previous review is resolved. The full Tier 1 implementation (22 tests, MODE +o/+v/+m/+t, KICK, NOTICE) remains solid. Ready to merge.
## Re-review: PR #88 — RPL_MYINFO fix
**Verdict: PASS ✅**
### Fix Verified
[`api.go:450`](https://git.eeqj.de/sneak/chat/src/branch/feature/tier1-channel-modes-85/internal/handlers/api.go#L450) now sends `"mnst"` (was `"imnst"`).
### Consistency Check
| Location | Channel Modes | Status |
|---|---|---|
| RPL_MYINFO (004) at `api.go:450` | `mnst` | ✅ |
| CHANMODES in 005 at `api.go:461` | `,,H,mnst` | ✅ |
| README line 1082 (004 example) | `mnst` | ✅ |
| README line 1083 (005 example) | `CHANMODES=,,H,mnst` | ✅ |
All four references are consistent. `+i` (invite-only) correctly excluded — it's [Tier 2 (#86)](https://git.eeqj.de/sneak/chat/issues/86).
### Build
`docker build .` passes — lint (0 issues), fmt-check, and all tests green. The rework commit (`3a6fc92`) is a single-line diff, no regressions.
### Summary
The blocking issue from the previous review is resolved. The full Tier 1 implementation (22 tests, MODE +o/+v/+m/+t, KICK, NOTICE) remains solid. Ready to merge.
I just ran make test on the PR branch (3a6fc92) — all tests pass, exit code 0. Also verified main (08f57bc) passes.
Could you paste the error output? I want to make sure I'm reproducing the same failure. Possible causes:
Missing web/dist/ placeholder files (the PR branch has an ensure-web-dist Makefile target that creates them automatically)
Cached build artifacts from a previous run
Different Go/toolchain version
I just ran `make test` on the PR branch (`3a6fc92`) — all tests pass, exit code 0. Also verified main (`08f57bc`) passes.
Could you paste the error output? I want to make sure I'm reproducing the same failure. Possible causes:
- Missing `web/dist/` placeholder files (the PR branch has an `ensure-web-dist` Makefile target that creates them automatically)
- Cached build artifacts from a previous run
- Different Go/toolchain version
Gitea CI shows commit 3a6fc92 failed after 2m48s. However, I ran docker build --no-cache . on a fresh clone of the PR branch and it passes cleanly:
Lint: 0 issues
fmt-check: pass
All tests: pass (handlers 20.2s, db 1.4s, hashcash 1.0s, ratelimit 1.0s, etc.)
Build: success
I can't access the CI job logs (API returns 403 for Actions, and the web UI requires owner auth). Could you paste the CI failure output? Possible CI-specific causes:
Runner resource constraints — 2m48s is tight if the runner has limited CPU/memory
Architecture difference — my Docker runs on ARM (M-series Mac); the CI runner is x86_64
Docker-in-Docker issue — the CI runner needs Docker available to run docker build .
Network/registry issue — pulling base images might have timed out on the runner
Gitea CI shows commit `3a6fc92` failed after 2m48s. However, I ran `docker build --no-cache .` on a fresh clone of the PR branch and it passes cleanly:
- Lint: 0 issues
- fmt-check: pass
- All tests: pass (handlers 20.2s, db 1.4s, hashcash 1.0s, ratelimit 1.0s, etc.)
- Build: success
I can't access the CI job logs (API returns 403 for Actions, and the web UI requires owner auth). Could you paste the CI failure output? Possible CI-specific causes:
1. **Runner resource constraints** — 2m48s is tight if the runner has limited CPU/memory
2. **Architecture difference** — my Docker runs on ARM (M-series Mac); the CI runner is x86_64
3. **Docker-in-Docker issue** — the CI runner needs Docker available to run `docker build .`
4. **Network/registry issue** — pulling base images might have timed out on the runner
sneak
merged commit 4b445e6383 into main2026-03-25 02:08:29 +01:00
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
Implement the core IRC channel functionality that users will immediately notice is missing. This is the foundation for all other mode enforcement.
closes #85
Changes
1. Channel Member Flags Schema
is_operator INTEGER NOT NULL DEFAULT 0andis_voiced INTEGER NOT NULL DEFAULT 0columns tochannel_memberstable2. MODE +o/+v/-o/-v (User Channel Modes)
MODE #channel +o nick/-o/+v/-vwith permission checks+ousers can grant/revoke modes@nickfor operators,+nickfor voiced usersPREFIX=(ov)@+3. MODE +m (Moderated)
is_moderated INTEGER NOT NULL DEFAULT 0tochannelstableERR_CANNOTSENDTOCHAN(404)4. MODE +t (Topic Lock)
is_topic_locked INTEGER NOT NULL DEFAULT 1tochannelstableERR_CHANOPRIVSNEEDED(482)5. KICK Command
KICK #channel nick [:reason]— operator-only6. NOTICE Differentiation
Additional Improvements
PREFIX=(ov)@+,CHANMODES=,,H,mnstTesting
22 new tests covering:
CI
docker build .passes — lint (0 issues), fmt-check, and all tests green.Review: PR #88 — Tier 1 Channel Modes (+o/+v/+m/+t), KICK, NOTICE
Verdict: FAIL
Build Status
docker build .passes — lint (0 issues), fmt-check, and all 22 new tests green. No build regressions.Requirements Checklist
is_operator INTEGER NOT NULL DEFAULT 0onchannel_members— ✅ schema correctis_voiced INTEGER NOT NULL DEFAULT 0onchannel_members— ✅ schema correct+oon first JOIN — ✅CountChannelMembers→JoinChannelAsOperator+o— ✅requireChannelOpgate@nickand+nickprefixes — ✅memberPrefix()functionPREFIX=(ov)@+— ✅ indeliverWelcome005 paramsis_moderated INTEGER NOT NULL DEFAULT 0on channels — ✅ schema correct+menforcement: only+o/+vcan send, others getERR_CANNOTSENDTOCHAN— ✅checkModeratedSendis_topic_locked INTEGER NOT NULL DEFAULT 1on channels — ✅ schema correct, default +t ON+tenforcement: only+ocan change topic — ✅executeTopicchecksIsChannelTopicLocked+ocheck, target removal, broadcast — ✅ full implementation withvalidateKick/broadcastKickcommand != irc.CmdNoticegate inhandleDirectMsg+Hchannels — ✅isNoticecheck beforevalidateChannelHashcashBlocking Issue: RPL_MYINFO (004) code/README mismatch
The README at line 1082 was updated to show:
But the actual code at
api.go:450still sends:The README says
"mnst"(noi), but the server sends"imnst"(withi). This is a factual documentation error — the README doesn't match the server's actual behavior.Furthermore, this creates an internal inconsistency within the code itself:
imnst(includesi),,H,mnst(does NOT includei)The same server simultaneously claims it supports
+iin one place and doesn't in another. The fix should be to updateapi.go:450to"mnst"to match both the README and the CHANMODES string.Code Quality Notes (non-blocking)
Overall, the implementation is well-structured. Specific positives:
requireChannelOp,resolveUserModeTarget,setChannelFlagare well-factored helpersfanOutSilentcleanup (removing unusedmetaparam) is a nice drive-by fixTestModeratedNoticeBlocked)fanOut) to support theparamsfield for target nick — correct approachMinor Observations (not blocking, for awareness)
+sin CHANMODES but unenforced:+s(secret) is still advertised inCHANMODES=,,H,mnstand RPL_MYINFO, but is documented as "Not yet enforced" and has no server-side handling. A client sendingMODE #channel +swould get an unknown mode error. This is pre-existing but worth noting since+iwas deliberately removed for the same reason.Missing
441and404from numeric reply reference table: The new KICK feature sends441(ERR_USERNOTINCHANNEL) and +m enforcement sends404(ERR_CANNOTSENDTOCHAN), but neither appears in the numeric codes reference table (lines 1080-1118). The KICK section documents 441, but the centralized table should be comprehensive for client developers. (404 absence is pre-existing.)Pre-existing:
setHashcashMode/clearHashcashModehave no operator check: The new mode handlers all correctly userequireChannelOp, but the pre-existing +H/-H handlers at lines 2798/2862 have no op check. Any channel member can set/clear hashcash. Not introduced by this PR.Summary
The implementation is solid — schema design, permission model, tests, and KICK broadcast logic are all correct. The single blocking issue is the RPL_MYINFO code/README mismatch at
api.go:450where"imnst"should be"mnst". One-line fix.Rework Complete
Fixed the RPL_MYINFO (004) channel mode string at
api.go:450:"imnst"(includedifor invite-only, which is Tier 2 / #86)"mnst"(matches CHANMODES in 005 and README)docker build .passes — lint, fmt-check, and all tests green.Re-review: PR #88 — RPL_MYINFO fix
Verdict: PASS ✅
Fix Verified
api.go:450now sends"mnst"(was"imnst").Consistency Check
api.go:450mnstapi.go:461,,H,mnstmnstCHANMODES=,,H,mnstAll four references are consistent.
+i(invite-only) correctly excluded — it's Tier 2 (#86).Build
docker build .passes — lint (0 issues), fmt-check, and all tests green. The rework commit (3a6fc92) is a single-line diff, no regressions.Summary
The blocking issue from the previous review is resolved. The full Tier 1 implementation (22 tests, MODE +o/+v/+m/+t, KICK, NOTICE) remains solid. Ready to merge.
“make test” fails
I just ran
make teston the PR branch (3a6fc92) — all tests pass, exit code 0. Also verified main (08f57bc) passes.Could you paste the error output? I want to make sure I'm reproducing the same failure. Possible causes:
web/dist/placeholder files (the PR branch has anensure-web-distMakefile target that creates them automatically)build is failing in gitea ci. how about “docker build .” on a fresh checkout?
Gitea CI shows commit
3a6fc92failed after 2m48s. However, I randocker build --no-cache .on a fresh clone of the PR branch and it passes cleanly:I can't access the CI job logs (API returns 403 for Actions, and the web UI requires owner auth). Could you paste the CI failure output? Possible CI-specific causes:
docker build .