feat(isupport): support draft/extended-isupport-0.2 with the KEY+= form#222
Open
ValwareIRC wants to merge 15 commits into
Open
feat(isupport): support draft/extended-isupport-0.2 with the KEY+= form#222ValwareIRC wants to merge 15 commits into
ValwareIRC wants to merge 15 commits into
Conversation
Adds first-class support for PushBot-style slash commands as defined by
the obbyircd doc/pushbot-spec.md protocol. The user types /forecast
london in #weather and the client:
1. Looks up "forecast" in the per-server botCommands cache (keyed
by lowercased bot nick).
2. If a bot in the current channel exposes that command, builds a
base64-encoded JSON payload { name, options } and sends
@+draft/bot-cmd=<b64> TAGMSG <#channel|botnick>, picking the
PRIVMSG-style "public" or NOTICE-style "private" wire form based
on the command's visibility field.
3. Falls back to the raw IRC command path if no match -- existing
/op, /me etc. behaviour is preserved.
Discovery is event-driven:
* draft/bot-cmds added to ourCaps so the server knows we're aware
of the protocol (informational; the server still does the work).
* registerPushBotHandlers (new src/store/handlers/pushbot.ts) hooks
TAGMSG, decodes +draft/bot-cmds responses, and writes to
server.botCommands. A +draft/bot-cmds-changed broadcast clears
the cached entry so the next slash invocation triggers a refetch.
* The handler is wired into registerAllHandlers in
src/store/handlers/index.ts.
Types extended:
* Server.botCommands: Record<botNick, BotCommand[]> on src/types.
* BotCommand / BotCommandOption mirror the JSON the bot publishes
(name, description, visibility, scopes, options[]).
Resolution order in tryDispatchBotCommand mirrors §7.5 of the spec:
explicit /cmd@botnick targets first, then channel-bots, then DM
partners, then server-wide bots. Public invocations go to the
channel (everyone sees the reply); private ones go to the bot with
+draft/channel-context to keep whisper-style replies routed to the
right view.
Tests: tests/store/pushbot.test.ts covers cache population,
+draft/bot-cmds-changed invalidation, and that unrelated TAGMSGs are
ignored. All 60 test files (789 tests) still pass.
After RPL_ENDOFWHO (315) for a channel, scan the channel's user list for users marked isBot=true (set by handleWhoxReply when the +B mode flag is present) and send a +draft/bot-cmds-query TAGMSG to each that we don't already have cached. Result: by the time the user types '/' in a channel with bots in it, the autocomplete list is already populated.
Two visible-to-the-user gaps now closed: 1. ChatArea rendered the slash popover from `cmdsAvailable` only. Merge in command names from `server.botCommands` so /forecast shows up in the popover when the user is in a channel with a bot that has registered it. 2. Discovery used to fire only on WHO_END. Add a lazy `queryUncachedBotsInChannel` triggered the first time the user starts a slash command in a channel that has +B users without cached schemas, so the popover catches up if WHO completed before the handler attached.
Previously the popover rendered every suggestion as just `/name`; the
caller didn't know whether the source was a server-side built-in or a
PushBot, and there was no signal that /forecast even takes arguments.
Now:
* `SlashSuggestion` carries name + description + options[] + source
({kind:"builtin"} or {kind:"bot", botNick, scope:"channel"|"server"}).
ChatArea builds these from cmdsAvailable (builtins) and from
server.botCommands (PushBot schemas).
* The popover renders the bare name plus a "channel-bot" / "server-bot"
badge with the bot nick, the description below, and `<required>` /
`[optional]` placeholders inline next to the name. Channel-bots
only appear when their bot is a member of the active channel;
server-wide bots show up everywhere we have a cached schema.
* A new SlashParamHint floats above the input once the user has typed
past the command name -- it highlights the active argument (the one
the cursor is currently in), shows the param's type, "required"
tag, description, and any `choices` list. Disappears for builtins
(no schema) and once the user has scrolled past all declared opts.
Tests: 6 new cases for getActiveParamContext covering cmd-name
typing, `//foo` escape, position 0..N argument tracking, `/cmd@bot`
targeting suffix, and case-insensitive cmd matching. All 61 test
files (795 tests) green.
The popover was missing the React-side commands (/me, /msg, /nick, /whisper, /join, /part, /away, /back) because they're handled locally before they touch the wire and never appear in the obsidianirc/cmdslist set. Centralized them in src/lib/clientCommands.ts with full schemas (description + options) so the popover and the param-hint render them identically to PushBot commands. Source kinds now distinguish: * client → handled locally; slate badge, "(handled by ObsidianIRC)" * server → from obsidianirc/cmdslist; emerald badge * bot → draft/bot-cmds; amber "channel-bot" or purple "server-bot" Dedup is client > server > bot, so /me always renders as client even if the server's cmdslist also advertises it. Badges have hover-tooltips explaining the source.
Two passes squashed:
(1) Negotiate the obby.world/channel-bots capability, receive the
server's bot directory burst at welcome time (BATCH wrapper, one
TAGMSG per bot carrying obby.world/bot-info=<base64-json>) plus
incremental add/update/remove pushes. State lands in server.bots
(Record<lowerNick, PushBotInfo>) and mirrors into botCommands so
the slash popover stays warm without a separate
+draft/bot-cmds-query.
(2) New BotsModal — left pane: filter (All / Server-wide / Channel) +
search + scope/status badges and online dot; right pane: realname,
transport, joined channels, command schemas, IRCop action buttons
(Approve / Suspend / Unsuspend / Delete) for non-config-defined
bots. Wired into ChatHeader via onOpenBots, both as a desktop
icon button (🤖, hidden md:block) and as an overflow-menu entry
for narrow/mobile views -- the first cut only added the overflow
entry which is invisible on desktop widths.
Tests: 2 new vitest cases for the bot-info pipeline (add populates
server.bots + botCommands; remove clears both). 61 test files,
797 tests green.
…cons The first cut of BotsModal was a one-off custom layout (flex split, non-portal, ad-hoc styling) using a 🤖 emoji as a header decoration. That worked but it didn't match the rest of the app and looked off on mobile. Rework it to mirror UserSettings: * useMediaQuery to branch desktop vs mobile * useModalBehavior for escape / click-outside * desktop: backdrop + centered card (max-w-4xl, h-80vh) with a fixed- width sidebar (filterable bot list) and a content pane (selected bot detail); Discord-dark palette and discord-primary accents. * mobile: full-screen createPortal with two views (list → detail drill-in, back button to return), safe-area padding matching UserSettings. Icons: every emoji used as UI chrome now uses react-icons/fa. * 🤖 channel-header button → <FaRobot /> * 🤖 overflow-menu entry → <FaRobot /> * online indicator dot in the bot list → <FaCircle /> * empty-state placeholder → <FaRobot className="text-4xl" /> Same surface area: filter chips, search input, status/scope badges, IRCop action buttons (Approve / Suspend / Unsuspend / Delete) for non-config-defined bots. The 🤖 next to bot nicknames in chat is unchanged -- that's a pre-existing bot identity marker, not UI chrome.
…dslist Two bugs: 1) Server-scope bots (helpbot, dicebot) never showed up in the picker. The picker skipped any bot that wasn't a channel member, which was right for channel-scope bots but wrong for server-scope bots that never auto-join. Now: gate the membership check on the bot's scope; server-scope bots are always offered. 2) When a server's cmdsAvailable advertised a name (e.g. HELP) that a bot also defined (helpbot's /help), the server entry won the dedup set and the bot was shadowed; the hint code meanwhile pulled the bot's schema for that name, producing the "picker says it's the built-in but the hint reads like the bot" mismatch. Process bot commands before cmdsAvailable so the canonical bot owner wins the seen-set, and apply the same scope filter to the hint schemas. Drive-by: replace `choices!.length` with `(choices?.length ?? 0)` in SlashParamHint that fix:unsafe had downgraded to an unsafe optional chain on the comparator.
Three UI/UX issues addressed: 1) Two bots can now publish the same command name and both show up as distinct entries in the picker. Cross-bot dedup is dropped, the existing `@<botNick>` badge differentiates rows visually. On select, the picker fills `/<cmd>@<botNick> ` so the existing useMessageSending `/cmd@bot` routing dispatches unambiguously. The bare-name reservation against server cmdsAvailable stays, so the built-in /HELP doesn't appear next to a bot's /help. 2) SlashCommandPopover now anchors via `bottom` (= input.top + 6px gap) instead of `top + estimated-height`. The earlier estimate used a per-row height that under-counted multi-line entries, so the popover drifted away from the input start when more rows or longer descriptions were present. `bottom` keeps it flush. 3) Selected-row highlight switched from bg-discord-text-link (a bright cyan that read as a hyperlink) to bg-discord-primary, matching the rest of the brand palette. Also: SlashParamHint now keeps the `@botnick` suffix in the returned cmdName so its schemas lookup tries `cmd@bot` first (specific) and falls back to `cmd` (bare); ChatArea's schemas map writes both keys per bot command. Test for getActiveParamContext updated to match.
The selected-row highlight is bg-discord-primary (blurple #5865F2), and the server-bot badge was bg-discord-primary/30 + text-discord- primary -- meaning the badge disappeared against its own selected row. Sky-700/40 + sky-300 reads as "network-wide service", contrasts every other badge state, and stays legible on both the default dark row and the selected blurple row.
When a bot publishes a command with declared options, picking it
from the slash popover now opens a one-per-option form modal
instead of putting the user on the hook to remember positional
arg ordering and free-form-type each value.
Renderer is type-driven from BotCommandOption.type:
string text input
int / number numeric input (step=1 / step=any)
bool checkbox
user text + datalist of channel members + DM partner
channel text + datalist of joined channels
date / time /
datetime native HTML picker
country select w/ ISO-3166-1 flag + name; wire value is
the alpha-2 code (lib/countries.ts)
password masked text input
any with choices[] select of the bot-declared choices (overrides
the type renderer)
useMessageSending grew an exported sendBotCommand() that takes a
pre-resolved bot + structured options map -- bypassing the
positional-arg parser used by the inline-typing path.
Translations for the 5 new strings added across all 18 non-English
locales.
Two related fixes around the slash-command UX:
1) The SlashParamHint stuck around after editing the command name to
something the schemas didn't recognise, because the hint was
rendered from messageTextRef.current (a ref, no re-render trigger)
while the parent intentionally skips re-renders on every keystroke
for perf. Subscribe to the input element's events from inside the
hint so it re-evaluates context independently and disappears as
soon as the cmd-name no longer matches.
2) Public slash commands are sent as a TAGMSG, so other channel
members only see the bot's reply with no context for what
triggered it. Render an attribution chip above the bot's reply
when it carries the new +obby.world/invoked-by tag -- a base64
JSON snapshot of {nick, name, options} produced server-side at
dispatch time.
The server-side half of (2) lands in pushbot.c: PbInteraction gains
an invoker_cmd_b64 field built during pb_dispatch_command, and
pb_make_reply_tags appends +obby.world/invoked-by alongside the
existing +reply / +draft/channel-context tags.
Two related changes pair with the server-side discovery split: 1) Each slash command in BotDetail is now a button; clicking it opens SlashCommandParamModal with the bot + command pre-set. Submit dispatches via the existing sendBotCommand path, which routes by command.visibility (public -> current channel, private -> DM to the bot with channel-context). BotsModal closes when a command is picked so the param modal isn't stacked underneath. 2) The bot list filters by reachability so the user only sees bots they can actually invoke right now: server-scope always, channel-scope only if any of bot.channels overlaps a channel they're a member of. Opers bypass the filter for management. Companion server-side change (pushbot.c) restricts the connect burst to server-scope bots only; channel-scope arrives on the relevant LOCAL_JOIN. Opers still get every bot in the burst.
The draft (https://github.com/ircv3/ircv3-specifications) defines a parallel 0.2 cap that adds an append form: KEY=value replace (existing) KEY+=chunk append; byte-wise concatenation onto held value -KEY delete This lets a server deliver a list-valued token whose value would otherwise exceed one RPL_ISUPPORT line by splitting it across multiple lines. The original draft/extended-isupport cap stays requested for backward compat with servers that haven't moved. Changes: - IRCClient.ourCaps grows draft/extended-isupport-0.2; the pre-registration ISUPPORT shove fires when either cap is in the requested set. - ircUtils.parseIsupport now wraps a new parseIsupportTokens() that returns ordered tokens with op kind ("set" | "append" | "delete"). Existing callers of parseIsupport keep working -- it flattens to the same key->value Record after applying op semantics. - IRCClientContext gains isupportValues: Map<serverId, Map<key, value>> so the connection.ts handler can accumulate across separate RPL_ISUPPORT lines for the same key. handleIsupport emits ISUPPORT events with the cumulative value each time it changes, so downstream consumers (protocol/isupport.ts) don't have to know about append vs set themselves. Tests for parseIsupportTokens cover all four forms, decoding, and the append-onto-unset edge case.
Contributor
📝 WalkthroughWalkthroughThis PR adds comprehensive IRC bot (PushBot) support to ObsidianIRC by integrating bot command discovery, parameterized command execution, and operator management. It unifies slash-command suggestions from client, server, and bot sources; adds parameter-entry and bot-browsing modals; updates IRC protocol handling for ISUPPORT token parsing; and provides bot invocation attribution in message display. ChangesPushBot Bot Command System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/locales/en/messages.po (1)
1-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegenerate and commit the i18n catalog to fix CI drift.
The i18n pipeline is failing because
npm run i18n:extractchangessrc/locales/en/messages.po, which means the checked-in catalog is out of sync with source strings.Please run and commit:
npm run i18n:extract && npm run i18n:compile🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/locales/en/messages.po` around lines 1 - 13, The i18n catalog file src/locales/en/messages.po is out of sync; run the extraction and compile scripts (npm run i18n:extract && npm run i18n:compile) to regenerate messages.po and compiled catalogs, then stage and commit the updated src/locales/en/messages.po (and any generated locale artifacts) so the checked-in catalog matches source strings and CI no longer drifts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/layout/ChatArea.tsx`:
- Around line 2411-2415: The bot visibility check incorrectly compares
case-sensitive botNick to a lowercased chanUsers set and only gates
channel-scoped bots when selectedChannel exists; update the logic in both the
suggestion and schema paths (locations using botScope, chanUsers, botNick,
selectedChannel) to first normalize botNick (e.g., toLowerCase()) before
membership testing against chanUsers, and always enforce the channel-scoped
exclusion (i.e., if botScope === "channel" and the normalized botNick is not in
chanUsers, skip the bot regardless of selectedChannel/DM context).
In `@src/components/ui/BotsModal.tsx`:
- Around line 374-377: The search filter in BotsModal (the expression that
checks query ? b.nick.toLowerCase()... : true) can crash when b.realname is
null/undefined; update the filter to guard b.realname (e.g., use optional
chaining or default to empty string) so the realname check becomes safe (for
example use b.realname?.toLowerCase() or (b.realname || '').toLowerCase() before
calling includes with query.toLowerCase()), keeping the existing b.nick check
and query logic intact.
In `@src/components/ui/SlashCommandParamModal.tsx`:
- Around line 368-391: The JSX contains hard-coded, non-translated UI strings in
SlashCommandParamModal (e.g., placeholders "nick" and "`#channel`" and raw label
text "required") — update the component to use `@lingui` i18n: wrap visible label
text children with <Trans>…</Trans> and replace prop strings (placeholder,
aria-label, title, etc.) with translated values from useLingui()/t`…` (e.g.,
const { i18n } = useLingui() or const { t } = useLingui() and use t`nick` /
t`#channel`), and adjust the required label text (lines referenced around the
required checkbox) to use <Trans> or t as appropriate so all displayed strings
in the fields (including those in the "string", "nick", and "channel" cases) are
localized.
- Around line 109-116: The int branch in SlashCommandParamModal.tsx currently
uses parseInt which accepts "1.9" as 1; change the validation in the o.type ===
"int" case to reject non-integer input: parse the raw value using Number(raw)
(or test the raw string with a strict integer regex like /^[+-]?\d+$/), check
Number.isFinite and Number.isInteger (or regex match) and if it fails call
setError(t`${o.name} must be a whole number.`) and return; only when the value
is confirmed an integer assign payload[o.name] = Number(raw) (or the parsed
integer) so decimals are not silently truncated.
- Around line 56-67: The component-level state values is only initialized once
and doesn't update when the incoming command/options change; in
SlashCommandParamModal reset values by deriving the same initial object (based
on opts and option types) inside an effect that runs when command or
command.options (opts) changes and call setValues with that new init object;
keep the same mapping rules (bool => false, int/number => "", else => "") and
update any dependent form inputs to use values from state so the modal shows the
newly selected command's defaults.
In `@src/components/ui/SlashCommandPopover.tsx`:
- Around line 84-93: The popover's hardcoded strings (labels/titles like
"client", "server" and header text "Slash commands") in SlashCommandPopover.tsx
must be internationalized: wrap visible JSX text children (e.g., the header
"Slash commands" and any inline label text) with <Trans>…</Trans> from
`@lingui/macro` and replace prop strings used for attributes (title, aria-label,
placeholder) with t`…` via useLingui()/t; update the return objects for the
"client" and "server" cases (the label and title properties) and any other
hardcoded text at the noted ranges (around lines 103-110 and 205-206) to use
these macros so all UI text is translated at runtime.
In `@src/components/ui/SlashParamHint.tsx`:
- Around line 148-152: In SlashParamHint.tsx, the user-facing literal strings
(e.g., the span that renders "(handled by ObsidianIRC)" and other literals like
"required" and "one of:" around lines 162-173) must be wrapped with the <Trans>
macro from `@lingui/macro` and the component should import Trans; update the
conditional render that uses entry.source === "bot" and the alternative text to
use <Trans> for the "(handled by ObsidianIRC)" fragment (and similarly wrap the
"required" and "one of:" text nodes in the JSX where they appear) so all visible
strings are localizable while preserving the existing logic and use of
entry.botNick.
- Around line 57-63: The argIndex counters in SlashParamHint.tsx are incorrectly
incremented for every space character; change the logic in the loop that
computes argIndex (using variables head, cursorInHead, argIndex) to only
increment when you encounter the start of a new space-run (i.e., a space whose
previous character is not a space or it's the first character), so consecutive
spaces count as a single separator and the argIndex correctly reflects argument
positions; keep the -1 meaning "still in cmd name region" and preserve the
existing early return when argIndex < 0.
In `@src/hooks/useMessageSending.ts`:
- Around line 119-121: The current b64 calculation uses
btoa(JSON.stringify(payload)) which breaks for non-Latin-1 characters; replace
it with a UTF-8-safe base64 encode of JSON.stringify(payload) so Unicode in
payload (options, names, channels) won't throw. Implement this by converting
JSON.stringify(payload) to a UTF-8 byte sequence (e.g., via TextEncoder or
equivalent), then build a binary string from that byte array and call btoa on
that binary string (then keep the trailing-padding strip .replace(/=+$/, "")).
Update the code that sets b64 (and any callers relying on it) to use this
UTF-8-safe routine.
In `@src/lib/clientCommands.ts`:
- Around line 27-114: The CLIENT_COMMANDS array currently contains module-scope
hardcoded English descriptions (fields on the objects in CLIENT_COMMANDS such as
the name "me" descriptions, "msg" descriptions, etc.), which breaks i18n
extraction; move creation of this array into a function (e.g. export function
getClientCommands()) so the user-facing description strings are created at
runtime inside a function body and wrap those strings with the t template
literal from `@lingui/macro` (import t) for each description and
option.description field; ensure you replace the module-level CLIENT_COMMANDS
export with a function that returns the same structure but with t`...` around
all translatable strings (option descriptions and command descriptions) so
extraction works and calls sites use getClientCommands().
In `@src/lib/irc/handlers/connection.ts`:
- Around line 124-130: The current tokenList construction filters only params
that start with ":" but the parser already strips that prefix so the ISUPPORT
sentinel "are supported by this server" remains and pollutes tokens; update the
filter used when building tokenList (the code referencing
parv.slice(1).filter(...) and tokenList) to exclude parameters equal to "are
supported by this server" (or that start with that phrase) in addition to
excluding ones that still start with ":" so the sentinel is removed before
joining into tokenList.
In `@src/lib/irc/IRCClient.ts`:
- Around line 555-557: The isupportValues Map stores per-server accumulated
ISUPPORT but is never cleared, causing stale keys to leak across server removals
or reconnects; update lifecycle handlers to remove or reinitialize entries keyed
by serverId: call isupportValues.delete(serverId) when a server is removed
(e.g., in removeServer/removeConnection handler) and ensure you create a fresh
map on new connections/reconnects (e.g., in connectServer/onReconnect) by
setting isupportValues.set(serverId, new Map()) or otherwise clearing the
existing Map for that serverId.
In `@src/locales/cs/messages.po`:
- Around line 25-27: The Czech locale file src/locales/cs/messages.po has many
empty translations (e.g., msgid "(suspended)" with msgstr ""), so run the
extraction script (npm run i18n:extract) to list all missing strings and then
populate every empty msgstr for the listed msgids (examples: "(suspended)",
"offline", "gateway online", "Gateway connected", "Approve", "Unsuspend", "Run",
"Operator actions", "Delete bot {0}?...", "Bots", "Bots on this network",
"Search bots", "Slash commands", "Transport", "Source", "Webhook",
"Server-wide", "config-defined", "self-registered", "Bot hasn't registered any
slash commands yet.", "No bots registered on this network yet.", "Select a bot
on the left to see its commands and management actions.") in
src/locales/cs/messages.po and mirror the same completions across other
non-English .po files (German, Spanish, Finnish, French, Italian, Japanese)
referenced in the review; ensure each msgid has a non-empty msgstr before
committing.
In `@src/locales/de/messages.po`:
- Around line 25-27: The German .po file contains empty translations (e.g.,
msgid "(suspended)" from src/components/ui/BotsModal.tsx) and must be completed
before merging; open src/locales/de/messages.po and provide proper German
strings for every msgstr "" entry introduced by the bot/slash changes (including
the "(suspended)" entry), then run the project extraction/validation (npm run
i18n:extract) to ensure no other non-English .po files contain empty msgstr
values and commit the updated .po files.
In `@src/locales/es/messages.po`:
- Around line 25-27: The Spanish .po file has many empty translations (msgstr
"") causing untranslated UI; run the i18n extraction tool (npm run i18n:extract)
to list missing keys, then provide Spanish translations for every empty msgstr
entry referenced (start with the shown msgid "(suspended)" from
src/components/ui/BotsModal.tsx and the other listed msgids such as "All",
"Approve", "Bots on this network", "config-defined", etc.), updating
src/locales/es/messages.po and any other non-English .po files to fill all empty
msgstr values before committing.
In `@src/locales/fi/messages.po`:
- Around line 25-28: Several Finnish translations are left empty (e.g., msgid
"(suspended)" used in BotsModal.tsx and many other new msgid entries); run npm
run i18n:extract to locate all missing translations, then fill every empty
msgstr "" for the new entries (including the noted msgid "(suspended)" and the
other newly added msgids referenced in the review) with proper Finnish
translations before merging so the bot UI is fully localized.
In `@src/locales/fr/messages.po`:
- Around line 25-28: Several French translations are missing (e.g., the msgid
"(suspended)" coming from src/components/ui/BotsModal.tsx and many other msgid
entries listed), leaving UI text untranslated; run the i18n extraction and
populate the empty msgstr "" values across src/locales/fr/messages.po so each
msgid (start with "(suspended)" and then the other msgids referenced at ranges
255-258, 304-307, 431-446, etc.) has an accurate French translation; use npm run
i18n:extract to refresh the list, update each empty msgstr with the proper
French phrase, and save the .po file ensuring no msgstr "" entries remain for
these new bot-related strings.
In `@src/locales/it/messages.po`:
- Around line 25-27: Multiple Italian translations in src/locales/it/messages.po
are still empty (e.g., the msgid "(suspended)" and many other msgstr "" entries
noted); fill each empty msgstr with the correct Italian translation, running npm
run i18n:extract to verify missing keys and then update the corresponding msgstr
values (including the entries at the ranges you listed) so no user-visible
msgstr remains empty before merging.
In `@src/locales/ja/messages.po`:
- Around line 25-27: Several Japanese translations are missing (e.g. msgid
"(suspended)" used in src/components/ui/BotsModal.tsx) causing untranslated UI
strings; run the extraction script (npm run i18n:extract) to list all missing
keys, then edit src/locales/ja/messages.po to provide appropriate Japanese text
for each msgid indicated (including the ranges noted such as lines around the
BotsModal entries and the other msgid groups), ensuring every msgstr "" is
replaced with a correct translation and saving the .po file so the Japanese
locale renders fully translated strings.
---
Outside diff comments:
In `@src/locales/en/messages.po`:
- Around line 1-13: The i18n catalog file src/locales/en/messages.po is out of
sync; run the extraction and compile scripts (npm run i18n:extract && npm run
i18n:compile) to regenerate messages.po and compiled catalogs, then stage and
commit the updated src/locales/en/messages.po (and any generated locale
artifacts) so the checked-in catalog matches source strings and CI no longer
drifts.
🪄 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: b1c6d569-1904-4533-a6ac-213d064b4c0e
📒 Files selected for processing (60)
src/components/layout/ChatArea.tsxsrc/components/layout/ChatHeader.tsxsrc/components/message/BotInvocationChip.tsxsrc/components/message/MessageItem.tsxsrc/components/ui/BotsModal.tsxsrc/components/ui/SlashCommandParamModal.tsxsrc/components/ui/SlashCommandPopover.tsxsrc/components/ui/SlashParamHint.tsxsrc/hooks/useMessageSending.tssrc/lib/clientCommands.tssrc/lib/countries.tssrc/lib/irc/IRCClient.tssrc/lib/irc/IRCClientContext.tssrc/lib/irc/handlers/connection.tssrc/lib/ircUtils.tsxsrc/locales/cs/messages.mjssrc/locales/cs/messages.posrc/locales/de/messages.mjssrc/locales/de/messages.posrc/locales/en/messages.mjssrc/locales/en/messages.posrc/locales/es/messages.mjssrc/locales/es/messages.posrc/locales/fi/messages.mjssrc/locales/fi/messages.posrc/locales/fr/messages.mjssrc/locales/fr/messages.posrc/locales/it/messages.mjssrc/locales/it/messages.posrc/locales/ja/messages.mjssrc/locales/ja/messages.posrc/locales/ko/messages.mjssrc/locales/ko/messages.posrc/locales/nl/messages.mjssrc/locales/nl/messages.posrc/locales/pl/messages.mjssrc/locales/pl/messages.posrc/locales/pt/messages.mjssrc/locales/pt/messages.posrc/locales/ro/messages.mjssrc/locales/ro/messages.posrc/locales/ru/messages.mjssrc/locales/ru/messages.posrc/locales/sv/messages.mjssrc/locales/sv/messages.posrc/locales/tr/messages.mjssrc/locales/tr/messages.posrc/locales/uk/messages.mjssrc/locales/uk/messages.posrc/locales/zh-TW/messages.mjssrc/locales/zh-TW/messages.posrc/locales/zh/messages.mjssrc/locales/zh/messages.posrc/store/handlers/index.tssrc/store/handlers/pushbot.tssrc/types/index.tstests/components/SlashParamHint.test.tstests/components/layout/ChatHeader.memberButton.test.tsxtests/lib/isupportParse.test.tstests/store/pushbot.test.ts
Comment on lines
+2411
to
+2415
| const botScope: "channel" | "server" = | ||
| srv.bots?.[botNick]?.scope ?? "channel"; | ||
| const inChannel = chanUsers.has(botNick); | ||
| if (botScope === "channel" && selectedChannel && !inChannel) | ||
| continue; |
Contributor
There was a problem hiding this comment.
Channel-bot visibility logic is incorrect (case + DM gating).
Two issues in both suggestion and schema paths:
chanUsersis lowercased, but membership checks use rawbotNick(case mismatch risk).- Channel-scoped bots are only filtered when
selectedChannelexists, so they can still appear in DM context.
Suggested fix pattern (apply in both blocks)
- const inChannel = chanUsers.has(botNick);
- if (botScope === "channel" && selectedChannel && !inChannel) continue;
+ const inChannel = chanUsers.has(botNick.toLowerCase());
+ if (botScope === "channel" && (!selectedChannel || !inChannel)) continue;Also applies to: 2544-2548
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/layout/ChatArea.tsx` around lines 2411 - 2415, The bot
visibility check incorrectly compares case-sensitive botNick to a lowercased
chanUsers set and only gates channel-scoped bots when selectedChannel exists;
update the logic in both the suggestion and schema paths (locations using
botScope, chanUsers, botNick, selectedChannel) to first normalize botNick (e.g.,
toLowerCase()) before membership testing against chanUsers, and always enforce
the channel-scoped exclusion (i.e., if botScope === "channel" and the normalized
botNick is not in chanUsers, skip the bot regardless of selectedChannel/DM
context).
Comment on lines
+374
to
+377
| query | ||
| ? b.nick.toLowerCase().includes(query.toLowerCase()) || | ||
| b.realname.toLowerCase().includes(query.toLowerCase()) | ||
| : true, |
Contributor
There was a problem hiding this comment.
Search filter can crash when realname is missing.
Line 376 calls b.realname.toLowerCase() without a null/undefined guard, but realname is optional in other branches.
Suggested fix
- b.realname.toLowerCase().includes(query.toLowerCase())
+ (b.realname ?? "").toLowerCase().includes(query.toLowerCase())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/BotsModal.tsx` around lines 374 - 377, The search filter in
BotsModal (the expression that checks query ? b.nick.toLowerCase()... : true)
can crash when b.realname is null/undefined; update the filter to guard
b.realname (e.g., use optional chaining or default to empty string) so the
realname check becomes safe (for example use b.realname?.toLowerCase() or
(b.realname || '').toLowerCase() before calling includes with
query.toLowerCase()), keeping the existing b.nick check and query logic intact.
Comment on lines
+56
to
+67
| const opts = command.options ?? []; | ||
| const [values, setValues] = useState< | ||
| Record<string, string | number | boolean> | ||
| >(() => { | ||
| const init: Record<string, string | number | boolean> = {}; | ||
| for (const o of opts) { | ||
| if (o.type === "bool") init[o.name] = false; | ||
| else if (o.type === "int" || o.type === "number") init[o.name] = ""; | ||
| else init[o.name] = ""; | ||
| } | ||
| return init; | ||
| }); |
Contributor
There was a problem hiding this comment.
Form defaults won’t refresh when switching to another command.
values is initialized from opts only on first mount. If the modal stays mounted and command changes, fields/values can mismatch the newly selected command.
Suggested fix
+ useEffect(() => {
+ const init: Record<string, string | number | boolean> = {};
+ for (const o of opts) {
+ if (o.type === "bool") init[o.name] = false;
+ else init[o.name] = "";
+ }
+ setValues(init);
+ setError(null);
+ }, [command.name, opts]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/SlashCommandParamModal.tsx` around lines 56 - 67, The
component-level state values is only initialized once and doesn't update when
the incoming command/options change; in SlashCommandParamModal reset values by
deriving the same initial object (based on opts and option types) inside an
effect that runs when command or command.options (opts) changes and call
setValues with that new init object; keep the same mapping rules (bool => false,
int/number => "", else => "") and update any dependent form inputs to use values
from state so the modal shows the newly selected command's defaults.
Comment on lines
+109
to
+116
| if (o.type === "int") { | ||
| const n = Number.parseInt(String(raw), 10); | ||
| if (Number.isNaN(n)) { | ||
| setError(t`${o.name} must be a whole number.`); | ||
| return; | ||
| } | ||
| payload[o.name] = n; | ||
| } else if (o.type === "number") { |
Contributor
There was a problem hiding this comment.
int parsing currently truncates decimals instead of rejecting them.
parseInt("1.9", 10) returns 1, which silently changes user input. Integer fields should reject non-integer values.
Suggested fix
- if (o.type === "int") {
- const n = Number.parseInt(String(raw), 10);
- if (Number.isNaN(n)) {
+ if (o.type === "int") {
+ const n = Number(String(raw));
+ if (!Number.isInteger(n)) {
setError(t`${o.name} must be a whole number.`);
return;
}
payload[o.name] = n;📝 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.
Suggested change
| if (o.type === "int") { | |
| const n = Number.parseInt(String(raw), 10); | |
| if (Number.isNaN(n)) { | |
| setError(t`${o.name} must be a whole number.`); | |
| return; | |
| } | |
| payload[o.name] = n; | |
| } else if (o.type === "number") { | |
| if (o.type === "int") { | |
| const n = Number(String(raw)); | |
| if (!Number.isInteger(n)) { | |
| setError(t`${o.name} must be a whole number.`); | |
| return; | |
| } | |
| payload[o.name] = n; | |
| } else if (o.type === "number") { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/SlashCommandParamModal.tsx` around lines 109 - 116, The int
branch in SlashCommandParamModal.tsx currently uses parseInt which accepts "1.9"
as 1; change the validation in the o.type === "int" case to reject non-integer
input: parse the raw value using Number(raw) (or test the raw string with a
strict integer regex like /^[+-]?\d+$/), check Number.isFinite and
Number.isInteger (or regex match) and if it fails call setError(t`${o.name} must
be a whole number.`) and return; only when the value is confirmed an integer
assign payload[o.name] = Number(raw) (or the parsed integer) so decimals are not
silently truncated.
Comment on lines
+368
to
+391
| placeholder="nick" | ||
| list={listId} | ||
| autoComplete="off" | ||
| /> | ||
| <datalist id={listId}> | ||
| {choices.map((u) => ( | ||
| <option key={u} value={u} /> | ||
| ))} | ||
| </datalist> | ||
| </> | ||
| ); | ||
| break; | ||
| } | ||
| case "channel": { | ||
| const listId = `param-channels-${option.name}`; | ||
| field = ( | ||
| <> | ||
| <input | ||
| ref={autoFocusRef as React.RefObject<HTMLInputElement | null>} | ||
| type="text" | ||
| value={String(value ?? "")} | ||
| onChange={(e) => setValue(e.target.value)} | ||
| className={BASE_INPUT} | ||
| placeholder="#channel" |
Contributor
There was a problem hiding this comment.
Several field labels/placeholders are not translated.
Examples include placeholder="nick", placeholder="#channel", and raw required text in labels. These should be localized for non-English users.
As per coding guidelines, "Wrap JSX text children with <Trans>…</Trans> macro from @lingui/macro for i18n support" and "Use useLingui() with t template literal for JSX props like placeholder=, aria-label=, title= to enable i18n translation".
Also applies to: 422-425
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/SlashCommandParamModal.tsx` around lines 368 - 391, The JSX
contains hard-coded, non-translated UI strings in SlashCommandParamModal (e.g.,
placeholders "nick" and "`#channel`" and raw label text "required") — update the
component to use `@lingui` i18n: wrap visible label text children with
<Trans>…</Trans> and replace prop strings (placeholder, aria-label, title, etc.)
with translated values from useLingui()/t`…` (e.g., const { i18n } = useLingui()
or const { t } = useLingui() and use t`nick` / t`#channel`), and adjust the
required label text (lines referenced around the required checkbox) to use
<Trans> or t as appropriate so all displayed strings in the fields (including
those in the "string", "nick", and "channel" cases) are localized.
Comment on lines
+25
to
+27
| #: src/components/ui/BotsModal.tsx | ||
| msgid "(suspended)" | ||
| msgstr "" |
Contributor
There was a problem hiding this comment.
Translate all empty msgstr entries before committing.
24 bot-related UI strings have empty Spanish translations (msgstr ""). Users will see untranslated English text in the Spanish locale for:
(suspended),All,ApproveBot hasn't registered any slash commands yet.,Bots,Bots on this networkconfig-defined,Config-defined bot. Edit obbyircd.conf and /REHASH to change state.Delete bot {0}? This soft-deletes the database row; reuse the nick later only after a /REHASH.Gateway connected,gateway onlineNo bots registered on this network yet.Search bots,Select a bot on the left to see its commands and management actions.self-registered,Server-wideSlash commands,Source,Suspend,Transport,Unsuspendoffline,Webhook,Operator actions
As per coding guidelines, run npm run i18n:extract to identify missing strings, then fill in all empty msgstr "" entries across every non-English .po file before committing.
Also applies to: 255-257, 304-306, 431-433, 435-440, 442-445, 685-687, 689-691, 810-817, 1070-1072, 1074-1076, 1613-1615, 2138-2140, 2191-2193, 2207-2209, 2296-2298, 2397-2399, 2416-2418, 2447-2449, 2555-2557, 2593-2595, 2736-2738, 2783-2785, 2825-2827
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/locales/es/messages.po` around lines 25 - 27, The Spanish .po file has
many empty translations (msgstr "") causing untranslated UI; run the i18n
extraction tool (npm run i18n:extract) to list missing keys, then provide
Spanish translations for every empty msgstr entry referenced (start with the
shown msgid "(suspended)" from src/components/ui/BotsModal.tsx and the other
listed msgids such as "All", "Approve", "Bots on this network",
"config-defined", etc.), updating src/locales/es/messages.po and any other
non-English .po files to fill all empty msgstr values before committing.
Comment on lines
+25
to
+28
| #: src/components/ui/BotsModal.tsx | ||
| msgid "(suspended)" | ||
| msgstr "" | ||
|
|
Contributor
There was a problem hiding this comment.
Fill all newly added Finnish msgstr values before merge.
Many new user-visible entries are still msgstr "", so Finnish users will see untranslated strings in bot UI paths.
As per coding guidelines, “When adding new user-visible strings, translate them before committing — run npm run i18n:extract to identify missing strings, then fill in all empty msgstr "" entries across every non-English .po file before committing”.
Also applies to: 255-258, 304-307, 431-446, 685-692, 814-817, 1070-1077, 1613-1616, 1736-1739, 1825-1828, 2138-2141, 2191-2210, 2296-2299, 2396-2419, 2447-2450, 2555-2558, 2593-2596, 2783-2786
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/locales/fi/messages.po` around lines 25 - 28, Several Finnish
translations are left empty (e.g., msgid "(suspended)" used in BotsModal.tsx and
many other new msgid entries); run npm run i18n:extract to locate all missing
translations, then fill every empty msgstr "" for the new entries (including the
noted msgid "(suspended)" and the other newly added msgids referenced in the
review) with proper Finnish translations before merging so the bot UI is fully
localized.
Comment on lines
+25
to
+28
| #: src/components/ui/BotsModal.tsx | ||
| msgid "(suspended)" | ||
| msgstr "" | ||
|
|
Contributor
There was a problem hiding this comment.
Complete the missing French translations for new bot-related strings.
A significant set of new entries still has empty msgstr "", which leaves untranslated UI in French.
As per coding guidelines, “When adding new user-visible strings, translate them before committing — run npm run i18n:extract to identify missing strings, then fill in all empty msgstr "" entries across every non-English .po file before committing”.
Also applies to: 255-258, 304-307, 431-446, 685-692, 814-817, 1070-1077, 1613-1616, 1736-1739, 1825-1828, 2138-2141, 2191-2210, 2296-2299, 2396-2419, 2447-2450, 2555-2558, 2593-2596, 2783-2786
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/locales/fr/messages.po` around lines 25 - 28, Several French translations
are missing (e.g., the msgid "(suspended)" coming from
src/components/ui/BotsModal.tsx and many other msgid entries listed), leaving UI
text untranslated; run the i18n extraction and populate the empty msgstr ""
values across src/locales/fr/messages.po so each msgid (start with "(suspended)"
and then the other msgids referenced at ranges 255-258, 304-307, 431-446, etc.)
has an accurate French translation; use npm run i18n:extract to refresh the
list, update each empty msgstr with the proper French phrase, and save the .po
file ensuring no msgstr "" entries remain for these new bot-related strings.
Comment on lines
+25
to
+27
| #: src/components/ui/BotsModal.tsx | ||
| msgid "(suspended)" | ||
| msgstr "" |
Contributor
There was a problem hiding this comment.
Fill all newly added empty Italian translations before merge.
There are many newly introduced user-visible entries still set to msgstr "", which will surface untranslated copy in the Italian UI.
As per coding guidelines, "When adding new user-visible strings, translate them before committing — run npm run i18n:extract to identify missing strings, then fill in all empty msgstr "" entries across every non-English .po file before committing".
Also applies to: 255-257, 304-306, 431-446, 685-692, 814-817, 1070-1076, 1613-1615, 1736-1738, 1825-1827, 2138-2140, 2191-2193, 2207-2209, 2296-2298, 2396-2398, 2416-2418, 2447-2449, 2555-2557, 2593-2595, 2783-2785
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/locales/it/messages.po` around lines 25 - 27, Multiple Italian
translations in src/locales/it/messages.po are still empty (e.g., the msgid
"(suspended)" and many other msgstr "" entries noted); fill each empty msgstr
with the correct Italian translation, running npm run i18n:extract to verify
missing keys and then update the corresponding msgstr values (including the
entries at the ranges you listed) so no user-visible msgstr remains empty before
merging.
Comment on lines
+25
to
+27
| #: src/components/ui/BotsModal.tsx | ||
| msgid "(suspended)" | ||
| msgstr "" |
Contributor
There was a problem hiding this comment.
Complete all missing Japanese translations for the new strings.
A large set of newly added entries still have empty msgstr, so the Japanese locale will render untranslated text.
As per coding guidelines, "When adding new user-visible strings, translate them before committing — run npm run i18n:extract to identify missing strings, then fill in all empty msgstr "" entries across every non-English .po file before committing".
Also applies to: 255-257, 304-306, 431-446, 685-692, 814-817, 1070-1076, 1613-1615, 1736-1738, 1825-1827, 2138-2140, 2191-2193, 2207-2209, 2296-2298, 2396-2398, 2416-2418, 2447-2449, 2555-2557, 2593-2595, 2783-2785
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/locales/ja/messages.po` around lines 25 - 27, Several Japanese
translations are missing (e.g. msgid "(suspended)" used in
src/components/ui/BotsModal.tsx) causing untranslated UI strings; run the
extraction script (npm run i18n:extract) to list all missing keys, then edit
src/locales/ja/messages.po to provide appropriate Japanese text for each msgid
indicated (including the ranges noted such as lines around the BotsModal entries
and the other msgid groups), ensuring every msgstr "" is replaced with a correct
translation and saving the .po file so the Japanese locale renders fully
translated strings.
Multiple code paths can finalise CAP negotiation -- IRCClient.onCapAck (counter-driven), IRCClient.onCapLs (no-caps-to-request fast path), the 5-second CAP timeout, auth.ts's CAP ACK subscriber (per-ACK), SASL success/failure handlers, and LinkSecurityWarningModal. Most were guarded by their own local state but the auth.ts subscriber ran *per CAP ACK line*: when the server split a long CAP REQ into four ACK responses, it produced four CAP END + USER pairs in quick succession on the wire. Add a uniform `!capNegotiationComplete.get(serverId)` guard at every send site so CAP END / userOnConnect is a true one-shot regardless of which path wins. capNegotiationComplete is already cleared on disconnect/reconnect so future sessions still re-negotiate. The LinkSecurityWarningModal test mock grew a capNegotiationComplete Map to match the new contract; no production behaviour change for that path beyond becoming idempotent.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/irc/IRCClient.ts (1)
1858-1884:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope the CAP timeout to the connection attempt.
This timeout only keys off
serverId. If connection A drops and connection B reuses the sameserverIdbefore A's 5s timer fires, A's stale callback can delete B'spendingCapReqsentry and sendCAP END/USERon the replacement socket early.Suggested fix
- setTimeout(() => { + const capSocket = this.sockets.get(serverId); + setTimeout(() => { + if (this.sockets.get(serverId) !== capSocket) { + return; + } + if (this.pendingCapReqs.has(serverId)) { this.pendingCapReqs.delete(serverId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/irc/IRCClient.ts` around lines 1858 - 1884, The CAP timeout is currently keyed only by serverId, so a stale timer from an earlier connection can act on a new connection; modify the logic to scope the timeout to the specific connection attempt by capturing and verifying a unique connection token when scheduling the setTimeout. Create or use an existing per-connection identifier (e.g. a connectionToken or socketId stored alongside server entries), capture it into a local const before calling setTimeout, and inside the timer check that the current token for serverId still matches the captured token before deleting this.pendingCapReqs, sending "CAP END" via this.sendRaw, setting this.capNegotiationComplete, or calling this.userOnConnect; if it doesn’t match, simply return and do nothing. Ensure the same token is set/cleared when starting/tearing down connections so pendingCapReqs and saslEnabled/servers checks remain connection-scoped.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/irc/IRCClient.ts`:
- Around line 1858-1884: The CAP timeout is currently keyed only by serverId, so
a stale timer from an earlier connection can act on a new connection; modify the
logic to scope the timeout to the specific connection attempt by capturing and
verifying a unique connection token when scheduling the setTimeout. Create or
use an existing per-connection identifier (e.g. a connectionToken or socketId
stored alongside server entries), capture it into a local const before calling
setTimeout, and inside the timer check that the current token for serverId still
matches the captured token before deleting this.pendingCapReqs, sending "CAP
END" via this.sendRaw, setting this.capNegotiationComplete, or calling
this.userOnConnect; if it doesn’t match, simply return and do nothing. Ensure
the same token is set/cleared when starting/tearing down connections so
pendingCapReqs and saslEnabled/servers checks remain connection-scoped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74eadef4-c120-421d-8bda-fa89f86b6104
📒 Files selected for processing (5)
src/components/ui/LinkSecurityWarningModal.tsxsrc/lib/irc/IRCClient.tssrc/lib/irc/handlers/connection.tssrc/store/handlers/auth.tstests/components/LinkSecurityWarningModal.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/irc/handlers/connection.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the proposed v0.2 of the
extended-isupportdraft alongside the existingdraft/extended-isupportcap. The 0.2 draft adds theKEY+=valueappend form so a server can deliver a list-valued ISUPPORT token whose value exceeds oneRPL_ISUPPORTline by splitting it across multiple lines.Token grammar this branch now understands:
KEY=value— replaceKEY+=value— byte-wise concatenatevalueonto whatever's held forKEY-KEY— deleteThe original
draft/extended-isupportcap stays requested for back-compat with servers that haven't moved.Changes
IRCClient.ourCapsaddsdraft/extended-isupport-0.2; the pre-registrationISUPPORTshove fires when either cap is in the requested set.ircUtils.parseIsupportis now a thin wrapper around a newparseIsupportTokens()that returns ordered{ key, op, value }tokens. Existing callers keep working —parseIsupportflattens to the sameRecord<string, string>after applying op semantics.IRCClientContextgainsisupportValues: Map<serverId, Map<key, value>>soconnection.tscan accumulate+=chunks across separateRPL_ISUPPORTlines. TheISUPPORTevent downstream consumers subscribe to fires with the cumulative value each time it changes — they don't have to know about append vs set themselves.Test plan
npm run test -- --run tests/lib/isupportParse.test.tscovers all four forms (set, append, delete, flag),\x20decoding, append-onto-unset, and within-line flattening for the compat wrapper. 10/10 passing.npm run test -- --run— 806 passing, 1 skipped (unchanged from baseline).npm run buildclean.obbyircdbuild: connecting withdraft/extended-isupport-0.2+batchand issuingISUPPORTproduces adraft/isupportbatch withCLIENTTAGDENY=chunk1,CLIENTTAGDENY+=chunk2, …, and the client's accumulator yields the full concatenated value. v0.1-only clients still see the (truncated) single-line form, so cap gating works.The spec assigns no semantics to the byte-wise boundary between chunks, and the test server intentionally splits mid-name to exercise that — the accumulator joins them into the canonical comma-separated list.
Summary by CodeRabbit
New Features
Bug Fixes
Localization