feat: implement draft/whoami capability#178
Conversation
Implements the draft/whoami IRCv3 extension, which gives clients - Add draft/whoami to ourCaps (alongside the required chghost) - Add getUserFromNuh() and getHostFromNuh() to irc/utils.ts - Add myIdents/myHosts Maps to IRCClient + IRCClientContext; expose getMyIdent() / getMyHost() accessors; clean up on removeServer() - Extend SETNAME EventMap entry with optional ident?/host? fields - handleChghost: detect self-CHGHOST and update myIdents/myHosts (works with plain chghost too, not only when draft/whoami is active) - handleSetname: parse ident+host from source NUH and include in event so the store can initialise self-prefix from the draft/whoami registration-burst SETNAME - Add myIdent?/myHost? to the Server type - Store CHGHOST handler: set server.myIdent/myHost on self-CHGHOST - Store SETNAME handler: set server.myIdent/myHost from the registration-burst SETNAME when ident+host are present - 12 new tests covering utils, handleChghost, and handleSetname
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds per-server tracking of the client's ident and host learned from NUH-bearing messages. Introduces NUH parsing helpers, requests the "draft/whoami" capability, records ident/host from CHGHOST and SETNAME, exposes accessors, and persists values on the server type and store handlers. ChangesPer-server ident/host flow
sequenceDiagram
participant Client as IRCClient
participant Server as IRC Server
participant Store as App Store
participant Handlers as Lib Handlers
Client->>Server: Request capabilities (include "draft/whoami")
Server-->>Client: Capabilities ok
Server-->>Client: CHGHOST / SETNAME (source NUH or prefix)
Client->>Handlers: deliver raw message
Handlers->>Client: parse NUH (getUserFromNuh/getHostFromNuh)
Handlers->>Client: emit SETNAME/CHGHOST events (include ident/host if present)
Client->>Client: update myIdents/myHosts maps
Client->>Store: dispatch handler -> persist Server.myIdent/myHost
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Automated deployment preview for the PR in the Cloudflare Pages. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/lib/whoami.test.ts (1)
144-151: Minor::rrealname value doesn't reflect real production parv.In
handleMessageatsrc/lib/irc/IRCClient.tsthe trailing-parameter colon is stripped before the handler receivesparv(see lines 1416-1423), so in production aSETNAME :rmessage would yieldparv = ["r"], not[":r"]. The test still validates the join behavior, but consider passing["r"](and assertingrealname === "r") for realism.♻️ Proposed tweak
test("emits SETNAME event with user and realname", () => { const ctx = makeCtx("alice"); - handleSetname(ctx, "s1", ":alice!~u@bery6muzcsynw.irc", [":r"], undefined); + handleSetname(ctx, "s1", ":alice!~u@bery6muzcsynw.irc", ["r"], undefined); const data = ctx.events[0].data as { user: string; realname: string }; expect(ctx.events[0].event).toBe("SETNAME"); expect(data.user).toBe("alice"); - expect(data.realname).toBe(":r"); + expect(data.realname).toBe("r"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/lib/whoami.test.ts` around lines 144 - 151, The test in tests/lib/whoami.test.ts passes a trailing-parameter with the colon (parv = [":r"]) which production strips before handlers; update the test that calls handleSetname(ctx, "s1", ":alice!~u@bery6muzcsynw.irc", [":r"], undefined) to pass ["r"] instead and change the assertion to expect(data.realname).toBe("r") so the test reflects real-world behavior of the stripping in handleMessage/IRCClient.ts; keep the same test name and event checks.src/lib/irc/utils.ts (1)
6-15: LGTM — NUH parsing helpers handle the expected IRC prefix format correctly.Edge cases are well-covered: missing
!or@delimiters return"", and the optional leading colon is handled defensively. Tests attests/lib/whoami.test.ts:8-36provide good coverage.One minor note:
getHostFromNuhusessplit("@")[1]which returns only the segment between the first and second@. Standard IRC NUHs never contain multiple@so this is fine in practice, but if you want to be defensive against malformed input,withoutColon.slice(withoutColon.indexOf("@") + 1)would preserve any trailing@characters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/irc/utils.ts` around lines 6 - 15, getHostFromNuh currently uses withoutColon.split("@")[1] which can drop trailing '@' characters in malformed input; update getHostFromNuh to locate the first '@' via withoutColon.indexOf("@") and return withoutColon.slice(index + 1) (handling -1 to return "") so the host preserves any trailing '@' characters and remains defensive against malformed NUHs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/store/handlers/users.ts`:
- Around line 965-982: The isSelf branch currently returns after updating
currentUser and myIdent/myHost, leaving any User records for yourself inside
channel.users stale; modify the isSelf return to also update channel user
entries on the same server: when computing updatedServers (the mapping over
state.servers for s.id === serverId), in addition to setting myIdent/myHost, map
s.channels (or s.channels list) and for each channel map channel.users replacing
any user object whose nick equals ircClient.getNick(serverId) (the local `user`
variable) with a copy that has realname set to the new realname; then return
currentUser and this updatedServers so both currentUser.realname and
channel.users[*].realname for self are updated.
---
Nitpick comments:
In `@src/lib/irc/utils.ts`:
- Around line 6-15: getHostFromNuh currently uses withoutColon.split("@")[1]
which can drop trailing '@' characters in malformed input; update getHostFromNuh
to locate the first '@' via withoutColon.indexOf("@") and return
withoutColon.slice(index + 1) (handling -1 to return "") so the host preserves
any trailing '@' characters and remains defensive against malformed NUHs.
In `@tests/lib/whoami.test.ts`:
- Around line 144-151: The test in tests/lib/whoami.test.ts passes a
trailing-parameter with the colon (parv = [":r"]) which production strips before
handlers; update the test that calls handleSetname(ctx, "s1",
":alice!~u@bery6muzcsynw.irc", [":r"], undefined) to pass ["r"] instead and
change the assertion to expect(data.realname).toBe("r") so the test reflects
real-world behavior of the stripping in handleMessage/IRCClient.ts; keep the
same test name and event checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fac8915f-a68d-40a9-b578-5ebf308a0a38
📒 Files selected for processing (7)
src/lib/irc/IRCClient.tssrc/lib/irc/IRCClientContext.tssrc/lib/irc/handlers/users.tssrc/lib/irc/utils.tssrc/store/handlers/users.tssrc/types/index.tstests/lib/whoami.test.ts
| const isSelf = user === ircClient.getNick(serverId); | ||
|
|
||
| if (isSelf) { | ||
| // Self-SETNAME: update currentUser realname and, if we have prefix info from | ||
| // the draft/whoami registration burst, record myIdent/myHost on the server. | ||
| const updatedServers = | ||
| ident && host | ||
| ? state.servers.map((s) => | ||
| s.id === serverId ? { ...s, myIdent: ident, myHost: host } : s, | ||
| ) | ||
| : state.servers; | ||
| return { | ||
| currentUser: { | ||
| ...state.currentUser, | ||
| realname: realname, | ||
| }, | ||
| currentUser: state.currentUser | ||
| ? { ...state.currentUser, realname } | ||
| : state.currentUser, | ||
| servers: updatedServers, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Self-SETNAME branch skips updating channel.users[*].realname for self.
When isSelf is true, the handler returns early after updating only currentUser.realname and the optional server myIdent/myHost. It never falls through to the channel-user mapping at lines 984-995, so the User entry representing yourself inside each channel's users array retains the stale realname. Other users see your updated realname (via their own SETNAME path), but your own UI views that source realname from channel.users (not currentUser) will not refresh.
🔧 Proposed fix to also update self in channel user lists
if (isSelf) {
// Self-SETNAME: update currentUser realname and, if we have prefix info from
// the draft/whoami registration burst, record myIdent/myHost on the server.
- const updatedServers =
- ident && host
- ? state.servers.map((s) =>
- s.id === serverId ? { ...s, myIdent: ident, myHost: host } : s,
- )
- : state.servers;
+ const updatedServers = state.servers.map((s) => {
+ if (s.id !== serverId) return s;
+ const updatedChannels = s.channels.map((c) => ({
+ ...c,
+ users: c.users.map((u) =>
+ u.username === user ? { ...u, realname } : u,
+ ),
+ }));
+ return {
+ ...s,
+ channels: updatedChannels,
+ ...(ident && host ? { myIdent: ident, myHost: host } : {}),
+ };
+ });
return {
currentUser: state.currentUser
? { ...state.currentUser, realname }
: state.currentUser,
servers: updatedServers,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isSelf = user === ircClient.getNick(serverId); | |
| if (isSelf) { | |
| // Self-SETNAME: update currentUser realname and, if we have prefix info from | |
| // the draft/whoami registration burst, record myIdent/myHost on the server. | |
| const updatedServers = | |
| ident && host | |
| ? state.servers.map((s) => | |
| s.id === serverId ? { ...s, myIdent: ident, myHost: host } : s, | |
| ) | |
| : state.servers; | |
| return { | |
| currentUser: { | |
| ...state.currentUser, | |
| realname: realname, | |
| }, | |
| currentUser: state.currentUser | |
| ? { ...state.currentUser, realname } | |
| : state.currentUser, | |
| servers: updatedServers, | |
| }; | |
| } | |
| const isSelf = user === ircClient.getNick(serverId); | |
| if (isSelf) { | |
| // Self-SETNAME: update currentUser realname and, if we have prefix info from | |
| // the draft/whoami registration burst, record myIdent/myHost on the server. | |
| const updatedServers = state.servers.map((s) => { | |
| if (s.id !== serverId) return s; | |
| const updatedChannels = s.channels.map((c) => ({ | |
| ...c, | |
| users: c.users.map((u) => | |
| u.username === user ? { ...u, realname } : u, | |
| ), | |
| })); | |
| return { | |
| ...s, | |
| channels: updatedChannels, | |
| ...(ident && host ? { myIdent: ident, myHost: host } : {}), | |
| }; | |
| }); | |
| return { | |
| currentUser: state.currentUser | |
| ? { ...state.currentUser, realname } | |
| : state.currentUser, | |
| servers: updatedServers, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/store/handlers/users.ts` around lines 965 - 982, The isSelf branch
currently returns after updating currentUser and myIdent/myHost, leaving any
User records for yourself inside channel.users stale; modify the isSelf return
to also update channel user entries on the same server: when computing
updatedServers (the mapping over state.servers for s.id === serverId), in
addition to setting myIdent/myHost, map s.channels (or s.channels list) and for
each channel map channel.users replacing any user object whose nick equals
ircClient.getNick(serverId) (the local `user` variable) with a copy that has
realname set to the new realname; then return currentUser and this
updatedServers so both currentUser.realname and channel.users[*].realname for
self are updated.
# Conflicts: # src/types/index.ts
Summary
Implements ircv3/ircv3-specifications#603
What changed
draft/whoamiadded toourCapsalongside the requiredchghostgetUserFromNuh()/getHostFromNuh()insrc/lib/irc/utils.tsIRCClient:myIdents/myHostsMaps per server;getMyIdent()/getMyHost()accessors; cleanup inremoveServer();SETNAMEevent extended with optionalident?/host?IRCClientContext: exposes the new Maps to protocol handlershandleChghost: detects self-CHGHOST and updatesctx.myIdents/ctx.myHosts— works with plainchghosteven withoutdraft/whoamihandleSetname: parses ident+host from the source NUH and includes them in the event, so the store can capture self-prefix from thedraft/whoamiregistration-burstSETNAMEServertype: new optionalmyIdent?/myHost?fieldsCHGHOSThandler: writesserver.myIdent/server.myHoston self-CHGHOSTSETNAMEhandler: initialisesserver.myIdent/server.myHostfrom the registration-burstSETNAMEwhenident+hostare present; self-SETNAME realname update now also works correctly whencurrentUserexiststests/lib/whoami.test.tsBehaviour with chghost-but-no-draft/whoami
Self-CHGHOST tracking (
myIdents/myHosts) is active wheneverchghostis negotiated, regardless of whether the server also advertisesdraft/whoami. The registration-burstSETNAMEwith ident/host is only sent by servers that supportdraft/whoami, so on those servers the self-prefix is known immediately after connection registration; on others it becomes known on the firstCHGHOST.Summary by CodeRabbit