Skip to content

feat: PushBot slash commands (draft/bot-cmds + +draft/bot-cmd)#220

Open
ValwareIRC wants to merge 13 commits into
mainfrom
feat/pushbot-client
Open

feat: PushBot slash commands (draft/bot-cmds + +draft/bot-cmd)#220
ValwareIRC wants to merge 13 commits into
mainfrom
feat/pushbot-client

Conversation

@ValwareIRC
Copy link
Copy Markdown
Contributor

@ValwareIRC ValwareIRC commented May 16, 2026

Summary

  • adds first-class client support for the draft/bot-cmds capability and the +draft/bot-cmd / +draft/bot-cmds-query / +draft/bot-cmds-changed IRCv3 message tags
  • the user types /forecast london in #weather, the client looks up forecast in the per-bot schema cache, base64-encodes { "name": "forecast", "options": { "city": "london" } }, and ships it as a @+draft/bot-cmd=<b64> TAGMSG #weather — the server proxies to the bot's gateway/webhook and the reply comes back as a normal PRIVMSG tagged +reply=<msgid>
  • discovery is automatic: after WHO/ENDOFWHO completes for a channel, we pre-query every +B user we now share the channel with so the autocomplete cache is warm by the time the user types /
  • 3 new vitest cases covering cache fill, +draft/bot-cmds-changed invalidation, and the unrelated-TAGMSG case; all 60 test files still pass

Spec

The wire protocol is documented in obbyircd doc/pushbot-spec.md §7. tl;dr:

Tag Direction Carrier Purpose
+draft/bot-cmds-query client → bot TAGMSG "Tell me your commands"
+draft/bot-cmds bot → client TAGMSG The base64-JSON commands list
+draft/bot-cmds-changed bot → channel TAGMSG Refetch — my commands changed
+draft/bot-cmd client → bot/channel TAGMSG Slash-command invocation

Test plan

  • connect to an obbyircd-with-pushbot server and confirm draft/bot-cmds shows in CAP LS / CAP ACK
  • join #weather (with a registered PushBot present), wait for ENDOFWHO, run /forecast london, see the bot's reply rendered as a normal PRIVMSG with +reply tag
  • run /forecast@weather london to confirm the explicit-target syntax routes to the named bot
  • DM a server-wide bot with /help, confirm the invocation goes via TAGMSG to the bot's nick rather than the channel
  • kill the bot mid-command, confirm we see a FAIL BOTCMD TIMEOUT … standard-reply after 3 s
  • verify slash commands the local user doesn't have access to still fall through to raw IRC (/op, /me, etc. unaffected)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Discover and invoke bot-provided slash commands with correct public/private routing, parameter modal, and execution feedback (invocation chip).
    • Bots modal to browse/manage bots, view slash-command schemas, and run operator actions.
    • Enhanced slash UX: merged client/server/bot suggestions with source badges, per-argument hints, and lazy bot discovery while typing.
    • Country list & flag support for parameter inputs.
  • Localization

    • Added bot- and slash-command UI translations across many locales.
  • Tests

    • Added tests for bot command caching and invalidation.

Review Change Stack

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.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds end-to-end IRCv3 draft/bot-cmds support: types and CAP, a pushbot store handler that decodes/caches TAGMSG schemas and prefetches via WHO_END, UI discovery (suggestions, param hints, Bots modal), dispatch from the message hook via base64 TAGMSG, and tests for caching/invalidation.

Changes

Bot Command Support

Layer / File(s) Summary
Bot command types & capability
src/types/index.ts, src/lib/irc/IRCClient.ts
Adds BotCommand/BotCommandOption types, Server.botCommands?: Record<string,BotCommand[]>, and requests draft/bot-cmds + obby.world/channel-bots during CAP.
PushBot store handler: decode, cache, invalidate, query
src/store/handlers/pushbot.ts, src/store/handlers/index.ts, tests/store/pushbot.test.ts
registerPushBotHandlers decodes +draft/bot-cmds (base64→JSON→BotCommand[]), caches per-server per-bot keyed by lowercased nick, clears on +draft/bot-cmds-changed, prefills via WHO_END, exposes queryBotCommands / queryUncachedBotsInChannel, and includes Vitest coverage for populate/invalidate/no-op cases.
Message sending: bot-command dispatch
src/hooks/useMessageSending.ts
Adds tryDispatchBotCommand to resolve bot target (explicit/@nick, channel, DM, server), parse positional args per option types, base64-encode payload (trim padding), and send TAGMSG to the appropriate target. Exports sendBotCommand and integrates into / command path to prevent fallback when dispatched.
Client command registry
src/lib/clientCommands.ts
Adds canonical CLIENT_COMMANDS and CLIENT_COMMAND_NAMES for client-handled slash commands and their option schemas used by suggestion UI and hint logic.
Chat UI: suggestions, param hint, Bots modal, header wiring
src/components/layout/ChatArea.tsx, src/components/layout/ChatHeader.tsx, src/components/ui/SlashCommandPopover.tsx, src/components/ui/SlashParamHint.tsx, src/components/ui/SlashCommandParamModal.tsx, src/components/ui/BotsModal.tsx
Lazy-seeds uncached bot schemas on first slash typing in channel, merges client/server/bot suggestions with scope metadata, updates suggestion rendering/selection to structured SlashSuggestion, displays inline parameter hints, provides a param-collection modal for bot options, and adds a Bots modal plus ChatHeader "Bots" wiring.
Bot invocation rendering
src/components/message/BotInvocationChip.tsx, src/components/message/MessageItem.tsx
Adds BotInvocationChip to decode/tag-render bot-invocation tags and integrates it into message header rendering.
Countries helper
src/lib/countries.ts
Adds COUNTRIES list and flagEmoji helper for country-select UI in param modal.
Localization updates
src/locales/*
Adds/updates message catalog payloads and .po entries across locales to cover new bots UI, modal validation messages, and bot-invocation strings.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • matheusfillipe

Poem

"🐇 I hopped into code and found a slash,
Commands and schemas in a quiet stash.
I cached, I queried, then I sent—so spry,
Base64 whispers beneath the IRC sky.
🥕🎉"

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pushbot-client

@github-actions
Copy link
Copy Markdown

Pages Preview
Preview URL: https://feat-pushbot-client.obsidianirc.pages.dev

Automated deployment preview for the PR in the Cloudflare Pages.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/hooks/useMessageSending.ts`:
- Around line 102-104: Replace the unsafe btoa(JSON.stringify(payload)) call
that fails on non-ASCII input with a UTF-8-safe encoding: serialize payload
using JSON.stringify, encode the resulting string with TextEncoder (e.g., new
TextEncoder().encode(...)), convert the resulting Uint8Array to base64, and then
strip trailing '=' padding as before; update the b64 calculation in
useMessageSending.ts (the payload/b64 logic that builds the outgoing bot command
for cmd/options) to use this TextEncoder-based base64 path so Unicode (emojis,
non-Latin chars) no longer throw InvalidCharacterError.
- Around line 96-99: The integer parsing branch in useMessageSending.ts
currently does options[o.name] = Number.parseInt(raw, 10) which can yield NaN
that becomes null when JSON.stringify'd and corrupts the payload; change it to
parse first, check Number.isNaN(parsed) (or !Number.isFinite(parsed)) and if the
parse is invalid do not set that option (or set it to undefined/remove it) and
instead surface a validation error to the caller; use a temporary const parsed =
Number.parseInt(raw, 10) and only assign options[o.name] = parsed when
Number.isNaN(parsed) is false, otherwise skip assignment and trigger the
existing validation/error path in sendMessage/sendOptions handling.

In `@src/store/handlers/pushbot.ts`:
- Around line 17-25: decodeBotCmds currently uses atob() which produces a Latin1
string and corrupts multi-byte UTF-8 characters; change the implementation so
after adding padding you decode base64 into a byte array and then decode those
bytes as UTF-8 (e.g., use Uint8Array from the atob result or a base64-to-byte
utility and TextDecoder) before JSON.parse, then return parsed.commands as
before; update the function decodeBotCmds to perform: padded base64 → bytes →
new TextDecoder("utf-8").decode(bytes) → JSON.parse(decoded) and keep the
Array.isArray(parsed?.commands) check and return type unchanged.

In `@tests/store/pushbot.test.ts`:
- Around line 28-84: Add tests exercising the slash-dispatch/command-routing in
the useMessageSending hook: write new specs that mount or call useMessageSending
(or the exposed dispatchSlashCommand behavior) to assert that messages starting
with "/" are encoded/routed via dispatchSlashCommand to the correct server
botCommands (use encodeBotCmds to prepare mtags and ircClient.triggerEvent to
simulate TAGMSGs), verify both successful dispatch and fallback passthrough, and
ensure any effects clean up nested requestAnimationFrame calls (tests use RAF
mocked as setTimeout(cb,0) so cancel timers/RAF in teardown). Locate tests by
referencing useMessageSending, dispatchSlashCommand, encodeBotCmds, and
ircClient.triggerEvent to add the assertions.
🪄 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: f7c1c873-7d20-4eea-aa04-ef0d4a2cfc52

📥 Commits

Reviewing files that changed from the base of the PR and between e153b25 and 9f34253.

📒 Files selected for processing (6)
  • src/hooks/useMessageSending.ts
  • src/lib/irc/IRCClient.ts
  • src/store/handlers/index.ts
  • src/store/handlers/pushbot.ts
  • src/types/index.ts
  • tests/store/pushbot.test.ts

Comment on lines +96 to +99
if (o.type === "int") options[o.name] = Number.parseInt(raw, 10);
else if (o.type === "bool")
options[o.name] = raw === "true" || raw === "1" || raw === "yes";
else options[o.name] = raw;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n src/hooks/useMessageSending.ts | head -120

Repository: ObsidianIRC/ObsidianIRC

Length of output: 5229


🏁 Script executed:

rg -A 3 "tryDispatchBotCommand" src/ | head -50

Repository: ObsidianIRC/ObsidianIRC

Length of output: 549


🏁 Script executed:

rg -B 5 -A 10 "tryDispatchBotCommand\(" src/hooks/useMessageSending.ts | tail -40

Repository: ObsidianIRC/ObsidianIRC

Length of output: 1065


🏁 Script executed:

rg -B 5 -A 15 "type BotCommand" src/

Repository: ObsidianIRC/ObsidianIRC

Length of output: 49


🏁 Script executed:

rg "BotCommand" src/types.ts -B 2 -A 10

Repository: ObsidianIRC/ObsidianIRC

Length of output: 120


🏁 Script executed:

fd -e ts -e tsx "types" src/ | head -20

Repository: ObsidianIRC/ObsidianIRC

Length of output: 152


🏁 Script executed:

rg "BotCommand" src/store/types.ts -B 2 -A 10

Repository: ObsidianIRC/ObsidianIRC

Length of output: 49


🏁 Script executed:

rg "BotCommand" src/ -B 2 -A 10

Repository: ObsidianIRC/ObsidianIRC

Length of output: 8122


Guard invalid integer option parsing before dispatch.

If an int arg is invalid, Number.parseInt() yields NaN, which JSON.stringify converts to null, corrupting the payload sent to the bot.

Proposed fix
-    if (o.type === "int") options[o.name] = Number.parseInt(raw, 10);
+    if (o.type === "int") {
+      const parsedInt = Number.parseInt(raw, 10);
+      if (Number.isNaN(parsedInt)) return false;
+      options[o.name] = parsedInt;
+    }
📝 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") options[o.name] = Number.parseInt(raw, 10);
else if (o.type === "bool")
options[o.name] = raw === "true" || raw === "1" || raw === "yes";
else options[o.name] = raw;
if (o.type === "int") {
const parsedInt = Number.parseInt(raw, 10);
if (Number.isNaN(parsedInt)) return false;
options[o.name] = parsedInt;
}
else if (o.type === "bool")
options[o.name] = raw === "true" || raw === "1" || raw === "yes";
else options[o.name] = raw;
🤖 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/hooks/useMessageSending.ts` around lines 96 - 99, The integer parsing
branch in useMessageSending.ts currently does options[o.name] =
Number.parseInt(raw, 10) which can yield NaN that becomes null when
JSON.stringify'd and corrupts the payload; change it to parse first, check
Number.isNaN(parsed) (or !Number.isFinite(parsed)) and if the parse is invalid
do not set that option (or set it to undefined/remove it) and instead surface a
validation error to the caller; use a temporary const parsed =
Number.parseInt(raw, 10) and only assign options[o.name] = parsed when
Number.isNaN(parsed) is false, otherwise skip assignment and trigger the
existing validation/error path in sendMessage/sendOptions handling.

Comment on lines +102 to +104
const payload = { name: cmd.name, options };
const b64 = btoa(JSON.stringify(payload)).replace(/=+$/, ""); // strip trailing padding for IRCv3 tag-value friendliness
const isPublic = cmd.visibility !== "private";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, find and read the useMessageSending.ts file to understand context
fd -t f "useMessageSending.ts" --exec wc -l {} \;

Repository: ObsidianIRC/ObsidianIRC

Length of output: 104


🏁 Script executed:

# Read the file to see the full context around lines 102-104
fd -t f "useMessageSending.ts" --exec cat -n {} \; | head -130 | tail -40

Repository: ObsidianIRC/ObsidianIRC

Length of output: 1795


🏁 Script executed:

# Also search for where options object is populated or constrained
rg -B 5 -A 15 "tryDispatchBotCommand" src/hooks/useMessageSending.ts

Repository: ObsidianIRC/ObsidianIRC

Length of output: 1362


🌐 Web query:

JavaScript btoa Unicode error behavior throws exception

💡 Result:

The JavaScript btoa function is designed to create a Base64-encoded ASCII string from a binary string, where each character is treated as a single byte of data [1][2][3]. Because JavaScript strings are encoded as UTF-16, btoa expects every character in the input string to have a code point less than 256 (the Latin-1 range) [1][4][3]. If you pass a string containing characters with code points of 256 or higher (such as emojis, non-Latin characters, or certain symbols), btoa will throw an InvalidCharacterError DOMException [1][5][3]. This behavior is intentional and stems from the function's definition as a "binary-to-ASCII" converter, which does not natively support multi-byte Unicode characters [6][4][7]. To safely encode Unicode strings, you must first convert the string into a sequence of UTF-8 bytes and then represent those bytes as a binary string [1][4][3]. The recommended modern approach is to use the TextEncoder API [1][5][3]: function encodeBase64(str) { const encoder = new TextEncoder; const bytes = encoder.encode(str); const binString = Array.from(bytes, (byte) => String.fromCodePoint(byte)).join(''); return btoa(binString); } To decode such strings, you should reverse the process using TextDecoder [4][7][3]: function decodeBase64(base64) { const binString = atob(base64); const bytes = Uint8Array.from(binString, (char) => char.codePointAt(0)); const decoder = new TextDecoder; return decoder.decode(bytes); } Older workarounds, such as using unescape(encodeURIComponent(str)), are also common but are generally less preferred than the modern TextEncoder/TextDecoder pattern [5][8][9].

Citations:


🏁 Script executed:

# Check if there's any error handling or validation for parseInt results
rg -B 2 -A 2 "Number.parseInt" src/hooks/useMessageSending.ts

Repository: ObsidianIRC/ObsidianIRC

Length of output: 343


🏁 Script executed:

# Look for the full function signature to understand where raw args come from
sed -n '80,110p' src/hooks/useMessageSending.ts

Repository: ObsidianIRC/ObsidianIRC

Length of output: 1297


Use UTF-8-safe base64 encoding for outgoing bot command payloads.

btoa(JSON.stringify(payload)) throws an InvalidCharacterError when options contain non-ASCII characters (Unicode, emojis, etc.), breaking bot command dispatch for valid user input.

Since string-type command parameters receive raw user input directly (line 95), any user providing non-Latin characters will trigger this error. The TextEncoder approach below is the modern, recommended pattern for encoding Unicode strings to base64.

Proposed fix
  const payload = { name: cmd.name, options };
-  const b64 = btoa(JSON.stringify(payload)).replace(/=+$/, ""); // strip trailing padding for IRCv3 tag-value friendliness
+  const json = JSON.stringify(payload);
+  const bytes = new TextEncoder().encode(json);
+  const binary = Array.from(bytes, (b) => String.fromCharCode(b)).join("");
+  const b64 = btoa(binary).replace(/=+$/, ""); // strip trailing padding for IRCv3 tag-value friendliness
📝 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
const payload = { name: cmd.name, options };
const b64 = btoa(JSON.stringify(payload)).replace(/=+$/, ""); // strip trailing padding for IRCv3 tag-value friendliness
const isPublic = cmd.visibility !== "private";
const payload = { name: cmd.name, options };
const json = JSON.stringify(payload);
const bytes = new TextEncoder().encode(json);
const binary = Array.from(bytes, (b) => String.fromCharCode(b)).join("");
const b64 = btoa(binary).replace(/=+$/, ""); // strip trailing padding for IRCv3 tag-value friendliness
const isPublic = cmd.visibility !== "private";
🤖 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/hooks/useMessageSending.ts` around lines 102 - 104, Replace the unsafe
btoa(JSON.stringify(payload)) call that fails on non-ASCII input with a
UTF-8-safe encoding: serialize payload using JSON.stringify, encode the
resulting string with TextEncoder (e.g., new TextEncoder().encode(...)), convert
the resulting Uint8Array to base64, and then strip trailing '=' padding as
before; update the b64 calculation in useMessageSending.ts (the payload/b64
logic that builds the outgoing bot command for cmd/options) to use this
TextEncoder-based base64 path so Unicode (emojis, non-Latin chars) no longer
throw InvalidCharacterError.

Comment thread src/store/handlers/pushbot.ts Outdated
Comment on lines +17 to +25
function decodeBotCmds(value: string): BotCommand[] | null {
try {
// unrealircd emits the value as base64-encoded JSON to avoid the
// pain of escaping IRCv3 tag-value characters. Add padding before
// decoding because spec §7.2 lets the bot strip trailing '='.
const padded = value + "=".repeat((4 - (value.length % 4)) % 4);
const json = atob(padded);
const parsed = JSON.parse(json);
if (Array.isArray(parsed?.commands)) return parsed.commands as BotCommand[];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# Check if the file exists and read the relevant section
fd pushbot.ts src/

Repository: ObsidianIRC/ObsidianIRC

Length of output: 97


🏁 Script executed:

# Read the pushbot.ts file to see the full context
cat -n src/store/handlers/pushbot.ts

Repository: ObsidianIRC/ObsidianIRC

Length of output: 4143


🏁 Script executed:

# Search for test files or usage of decodeBotCmds
rg -l "decodeBotCmds|pushbot" --type ts --type tsx

Repository: ObsidianIRC/ObsidianIRC

Length of output: 95


🏁 Script executed:

# Search for unrealircd bot command documentation or comments
rg "unrealircd|botcmd|BotCommand" --type ts -C 3

Repository: ObsidianIRC/ObsidianIRC

Length of output: 16927


🏁 Script executed:

# Search for any test cases or examples of bot commands with non-ASCII
rg "BotCommand|description|name" src --type ts -A 2 -B 1 | head -50

Repository: ObsidianIRC/ObsidianIRC

Length of output: 2728


🏁 Script executed:

# Check if there's any documentation or spec reference about the draft/bot-cmds format
rg -i "draft/bot-cmds|spec|specification" . --type ts -A 2 | head -40

Repository: ObsidianIRC/ObsidianIRC

Length of output: 2388


🌐 Web query:

draft/bot-cmds IRC specification UTF-8 base64 encoding

💡 Result:

There is no official IRC specification named "draft/bot-cmds" that defines UTF-8 or base64 encoding requirements. The term "draft/bot-cmds" appears to be a misunderstanding or a conflation of several distinct IRCv3 concepts: 1. Bot Mode: The IRCv3 Working Group maintains a specification for "Bot Mode" [1][2]. This specification standardizes how clients mark themselves as bots using a user mode (typically +B) and a corresponding ISUPPORT token (e.g., BOT=B) [1][2]. It also defines a "bot" message tag to identify messages sent by bots [1][2]. This specification does not mandate specific encoding for bot commands, nor does it use the name "draft/bot-cmds" [1][2]. 2. IRCv3 Drafts: In the IRCv3 ecosystem, the "draft/" prefix is used for work-in-progress specifications (e.g., "draft/multiline") [3][4]. While there are many draft specifications, "draft/bot-cmds" is not a recognized or active IRCv3 draft [5]. 3. Encoding and Base64: IRC itself is historically an ASCII-based protocol. While modern IRCv3 implementations often support UTF-8, this is typically handled via the UTF8ONLY capability or general server configuration rather than a specific "bot-cmds" draft [5]. Base64 encoding is sometimes used in IRC for transmitting binary data or obfuscating payloads (often in the context of CTCP or custom bot protocols), but it is not a standard requirement for general IRC bot commands [6][7][8]. If you are encountering "draft/bot-cmds" in a specific software implementation or documentation, it is likely a custom, vendor-specific, or project-internal naming convention rather than a part of the official IRC or IRCv3 protocol standards.

Citations:


🏁 Script executed:

# Read the pushbot test file to see if there are UTF-8 test cases
cat -n tests/store/pushbot.test.ts

Repository: ObsidianIRC/ObsidianIRC

Length of output: 2925


🏁 Script executed:

# Check if encodeBotCmds is defined in the test file or elsewhere
rg "encodeBotCmds" . --type ts -B 3 -A 5

Repository: ObsidianIRC/ObsidianIRC

Length of output: 1089


🏁 Script executed:

# Check for any actual JSON payloads or fixtures with non-ASCII
rg "forecast|description|name" tests/store/pushbot.test.ts -B 2 -A 2

Repository: ObsidianIRC/ObsidianIRC

Length of output: 642


Fix decodeBotCmds() to properly decode UTF-8 JSON from base64.

The current implementation uses atob() which returns a Latin1 string, causing non-ASCII command names or descriptions to corrupt when multi-byte UTF-8 sequences are interpreted as individual code points. This silently breaks bot commands with international characters (Cyrillic, CJK, emoji, etc.).

The test helper at tests/store/pushbot.test.ts:22–26 correctly shows the encoding pattern: UTF-8 → base64, so decoding must reverse this: base64 → bytes → UTF-8.

Proposed fix
 function decodeBotCmds(value: string): BotCommand[] | null {
   try {
     const padded = value + "=".repeat((4 - (value.length % 4)) % 4);
-    const json = atob(padded);
-    const parsed = JSON.parse(json);
+    const binary = atob(padded);
+    const bytes = Uint8Array.from(binary, (ch) => ch.charCodeAt(0));
+    const json = new TextDecoder().decode(bytes);
+    const parsed = JSON.parse(json);
     if (Array.isArray(parsed?.commands)) return parsed.commands as BotCommand[];
📝 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
function decodeBotCmds(value: string): BotCommand[] | null {
try {
// unrealircd emits the value as base64-encoded JSON to avoid the
// pain of escaping IRCv3 tag-value characters. Add padding before
// decoding because spec §7.2 lets the bot strip trailing '='.
const padded = value + "=".repeat((4 - (value.length % 4)) % 4);
const json = atob(padded);
const parsed = JSON.parse(json);
if (Array.isArray(parsed?.commands)) return parsed.commands as BotCommand[];
function decodeBotCmds(value: string): BotCommand[] | null {
try {
// unrealircd emits the value as base64-encoded JSON to avoid the
// pain of escaping IRCv3 tag-value characters. Add padding before
// decoding because spec §7.2 lets the bot strip trailing '='.
const padded = value + "=".repeat((4 - (value.length % 4)) % 4);
const binary = atob(padded);
const bytes = Uint8Array.from(binary, (ch) => ch.charCodeAt(0));
const json = new TextDecoder().decode(bytes);
const parsed = JSON.parse(json);
if (Array.isArray(parsed?.commands)) return parsed.commands as BotCommand[];
🤖 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/store/handlers/pushbot.ts` around lines 17 - 25, decodeBotCmds currently
uses atob() which produces a Latin1 string and corrupts multi-byte UTF-8
characters; change the implementation so after adding padding you decode base64
into a byte array and then decode those bytes as UTF-8 (e.g., use Uint8Array
from the atob result or a base64-to-byte utility and TextDecoder) before
JSON.parse, then return parsed.commands as before; update the function
decodeBotCmds to perform: padded base64 → bytes → new
TextDecoder("utf-8").decode(bytes) → JSON.parse(decoded) and keep the
Array.isArray(parsed?.commands) check and return type unchanged.

Comment on lines +28 to +84
describe("pushbot store handler", () => {
beforeEach(() => {
setupServer();
});

it("caches commands when TAGMSG carries +draft/bot-cmds", () => {
ircClient.triggerEvent("TAGMSG", {
serverId: "srv-1",
sender: "Weather",
channelName: "ourNick",
mtags: {
"+draft/bot-cmds": encodeBotCmds([
{ name: "forecast", description: "Look up the weather" },
]),
},
timestamp: new Date(),
});

const server = useStore.getState().servers[0];
expect(server.botCommands).toBeDefined();
expect(server.botCommands?.weather).toEqual([
{ name: "forecast", description: "Look up the weather" },
]);
});

it("invalidates the cache on +draft/bot-cmds-changed", () => {
useStore.setState((s) => ({
servers: s.servers.map((srv) => ({
...srv,
botCommands: { weather: [{ name: "forecast" }] },
})),
}));

ircClient.triggerEvent("TAGMSG", {
serverId: "srv-1",
sender: "Weather",
channelName: "ourNick",
mtags: { "+draft/bot-cmds-changed": "1" },
timestamp: new Date(),
});

const server = useStore.getState().servers[0];
expect(server.botCommands?.weather).toBeUndefined();
});

it("ignores TAGMSGs without bot-cmds tags", () => {
ircClient.triggerEvent("TAGMSG", {
serverId: "srv-1",
sender: "Weather",
channelName: "#general",
mtags: { "+typing": "active" },
timestamp: new Date(),
});
const server = useStore.getState().servers[0];
expect(server.botCommands).toBeUndefined();
});
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Add tests for slash-dispatch logic in useMessageSending.

This suite validates cache behavior, but the new command-routing/encoding path in src/hooks/useMessageSending.ts is still untested.

As per coding guidelines, "Add tests for all business logic, hooks, and media/URL handling — cancel nested requestAnimationFrame calls in effect cleanup to avoid post-unmount setState errors (RAF is mocked as setTimeout(cb, 0) in tests/setup.ts)."

🤖 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 `@tests/store/pushbot.test.ts` around lines 28 - 84, Add tests exercising the
slash-dispatch/command-routing in the useMessageSending hook: write new specs
that mount or call useMessageSending (or the exposed dispatchSlashCommand
behavior) to assert that messages starting with "/" are encoded/routed via
dispatchSlashCommand to the correct server botCommands (use encodeBotCmds to
prepare mtags and ircClient.triggerEvent to simulate TAGMSGs), verify both
successful dispatch and fallback passthrough, and ensure any effects clean up
nested requestAnimationFrame calls (tests use RAF mocked as setTimeout(cb,0) so
cancel timers/RAF in teardown). Locate tests by referencing useMessageSending,
dispatchSlashCommand, encodeBotCmds, and ircClient.triggerEvent to add the
assertions.

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.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/store/handlers/pushbot.ts (1)

37-37: 💤 Low value

Use optional chain as suggested by static analysis.

Proposed fix
-    if (!mask || !mask.startsWith("#")) return;
+    if (!mask?.startsWith("#")) return;
🤖 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/store/handlers/pushbot.ts` at line 37, Replace the null/undefined +
startsWith check with optional chaining: in the pushbot handler where the
variable mask is validated (the line currently reading if (!mask ||
!mask.startsWith("#")) return;), use mask?.startsWith(...) so the expression
short-circuits when mask is null/undefined and you only rely on startsWith when
mask is a string.
🤖 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.

Nitpick comments:
In `@src/store/handlers/pushbot.ts`:
- Line 37: Replace the null/undefined + startsWith check with optional chaining:
in the pushbot handler where the variable mask is validated (the line currently
reading if (!mask || !mask.startsWith("#")) return;), use mask?.startsWith(...)
so the expression short-circuits when mask is null/undefined and you only rely
on startsWith when mask is a string.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a3514ff9-76e1-4510-a305-0dd437d11953

📥 Commits

Reviewing files that changed from the base of the PR and between 9f34253 and be8a07b.

📒 Files selected for processing (2)
  • src/components/layout/ChatArea.tsx
  • src/store/handlers/pushbot.ts

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.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 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/ui/BotsModal.tsx`:
- Around line 33-69: STATUS_BADGE and SCOPE_BADGE currently hold user-facing
hardcoded English strings at module scope; move their creation into
component/runtime scope and localize using lingui's runtime API: inside the
component that renders these badges call useLingui() (or use the t`` template)
to produce localized label/title strings for the entries keyed by
PushBotInfo["status"] and PushBotInfo["scope"], or return them from helper
functions (e.g., getStatusBadge(status) / getScopeBadge(scope)) so no t`` calls
are at module scope and JSX props receive localized strings instead of raw
literals.

In `@src/components/ui/SlashCommandPopover.tsx`:
- Around line 80-110: The badgeStyle function returns user-visible labels and
tooltip titles as hard-coded strings; update badgeStyle to use Lingui's t
template literal (via useLingui()) for all label and title values (e.g., replace
"client", "server", "channel-bot", and their title strings with t`...`) and
ensure any JSX header text (the "Slash commands" header rendered elsewhere
around lines ~203-205) is wrapped with <Trans>…</Trans>; import/use useLingui()
to get t for prop strings (title=) and wrap visible JSX children with <Trans>.
Reference: badgeStyle (function) and the component header rendering of "Slash
commands". Ensure no plain English literals remain for these badges or header.

In `@src/components/ui/SlashParamHint.tsx`:
- Around line 108-133: Replace the raw English literals in SlashParamHint.tsx
with Lingui macros: wrap JSX text children like "(handled by ObsidianIRC)",
"required" and "one of:" in <Trans>…</Trans> and use useLingui()/t`` for any
prop/inline template strings (e.g., the `via @${entry.botNick}` interpolation)
so they are localizable; update imports to include Trans and useLingui/t and
adjust the JSX around entry.source, opts[ctx.argIndex], and
opts[ctx.argIndex].choices to use the localized strings instead of hardcoded
English.
- Around line 130-136: Replace the non-null assertions on choices by extracting
and guarding the value first: read the parameter into a local (e.g., const param
= opts[ctx.argIndex]) and then const choices = param?.choices; use a single
conditional like if (choices && choices.length > 0) (or in JSX: {choices &&
choices.length > 0 && (...)}) and reference choices.join(", ") inside that block
instead of opts[ctx.argIndex].choices!; this removes the forbidden `!` while
preserving the existing behavior in SlashParamHint.tsx.

In `@src/lib/clientCommands.ts`:
- Around line 27-114: The CLIENT_COMMANDS constant holds user-visible
descriptions at module scope and must be produced by a factory called at runtime
so localization via the t template literal is used inside function scope; create
a function (e.g., getClientCommands or buildClientCommands) that imports t from
'`@lingui/macro`' and returns the array previously in CLIENT_COMMANDS, replacing
every description and option.description with t`...` calls, then update call
sites to use getClientCommands() and remove the module-scope CLIENT_COMMANDS
constant to ensure i18n-safe construction for commands like "me", "msg",
"whisper", "join", "part", "nick", "away", and "back".

In `@src/locales/cs/messages.po`:
- Around line 25-27: The Czech .po file has many new msgid entries left with
empty msgstr (e.g., msgid "(suspended)" in src/locales/cs/messages.po and other
ranges listed), so fill in appropriate Czech translations for every msgid with
an empty msgstr across that file (including the ranges called out: 240-242,
289-292, etc.), ensuring plural forms and contexts are preserved if present and
that no non-English .po file contains msgstr "" before committing; update each
msgstr to the correct Czech string matching the English msgid.

In `@src/locales/de/messages.po`:
- Around line 25-28: Populate the empty German translations in
src/locales/de/messages.po for all newly added bot-related msgid entries (e.g.,
msgid "(suspended)" from src/components/ui/BotsModal.tsx and the other listed
untranslated msgids) by replacing each msgstr "" with the correct German text,
ensure plural forms if any are handled, save the file, then re-run the i18n
extract/compile step to validate no untranslated entries remain; focus on the
entries referenced in the review (the multiple blocks of untranslated bot
strings) and verify compilation succeeds.

In `@src/locales/es/messages.po`:
- Around line 25-27: The new Spanish .po entries for bot-related UI are
untranslated (msgstr "" for msgid "(suspended)" and other strings coming from
BotsModal and ChatHeader); open src/locales/es/messages.po, locate the msgid
entries emitted from the components BotsModal.tsx and ChatHeader (and any
adjacent empty msgstr entries), and provide accurate Spanish translations for
each msgstr (ensuring grammar/context matches the UI), then run npm run
i18n:extract to verify no missing translations remain before committing.

In `@src/locales/fi/messages.po`:
- Around line 25-27: The Finnish locale has empty translations for new
bot-management strings (e.g., msgid "(suspended)" used by BotsModal.tsx); run
the extraction task (npm run i18n:extract) to list all missing keys, then fill
every empty msgstr "" for those bot-related msgid entries in the fi/messages.po
(including "(suspended)" and the other new bot keys) so all non-English .po
files contain translations before committing.

In `@src/locales/it/messages.po`:
- Around line 26-27: The Italian locale file has many untranslated entries; run
`npm run i18n:extract` to refresh missing keys and then fill every empty msgstr
"" in src/locales/it/messages.po for the listed msgids (e.g. "(suspended)",
"All", "Approve", "Bot hasn't registered any slash commands yet.", "Bots", "Bots
on this network", "config-defined", "Config-defined bot. Edit obbyircd.conf and
/REHASH to change state.", "offline", the "Delete bot {0}?  This
soft-deletes..." string, "Gateway connected", "gateway online", "No bots
registered on this network yet.", "Operator actions", "Search by nick or
description", "Select a bot on the left to see its commands and management
actions.", "self-registered", "Server-wide", "Slash commands", "Source",
"Suspend", "Unsuspend", "Webhook") providing proper Italian translations for
each msgid so no UI strings remain in English before committing.

In `@src/locales/ko/messages.po`:
- Around line 25-28: The Korean locale file has many empty translations (e.g.,
the msgid "(suspended)" entry and numerous other msgstr "" entries) causing
fallback/mixed-language UI; run the i18n extraction (npm run i18n:extract), then
open src/locales/ko/messages.po and fill each empty msgstr for the listed msgid
entries (including "(suspended)" and the ranges noted) with correct Korean
translations, saving the file so no msgstr remains empty before merging.
🪄 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: 78aad2bb-33d5-4829-8f0d-bcd904c683dc

📥 Commits

Reviewing files that changed from the base of the PR and between be8a07b and 4de4db6.

📒 Files selected for processing (50)
  • src/components/layout/ChatArea.tsx
  • src/components/layout/ChatHeader.tsx
  • src/components/ui/BotsModal.tsx
  • src/components/ui/SlashCommandPopover.tsx
  • src/components/ui/SlashParamHint.tsx
  • src/lib/clientCommands.ts
  • src/lib/irc/IRCClient.ts
  • src/locales/cs/messages.mjs
  • src/locales/cs/messages.po
  • src/locales/de/messages.mjs
  • src/locales/de/messages.po
  • src/locales/en/messages.mjs
  • src/locales/en/messages.po
  • src/locales/es/messages.mjs
  • src/locales/es/messages.po
  • src/locales/fi/messages.mjs
  • src/locales/fi/messages.po
  • src/locales/fr/messages.mjs
  • src/locales/fr/messages.po
  • src/locales/it/messages.mjs
  • src/locales/it/messages.po
  • src/locales/ja/messages.mjs
  • src/locales/ja/messages.po
  • src/locales/ko/messages.mjs
  • src/locales/ko/messages.po
  • src/locales/nl/messages.mjs
  • src/locales/nl/messages.po
  • src/locales/pl/messages.mjs
  • src/locales/pl/messages.po
  • src/locales/pt/messages.mjs
  • src/locales/pt/messages.po
  • src/locales/ro/messages.mjs
  • src/locales/ro/messages.po
  • src/locales/ru/messages.mjs
  • src/locales/ru/messages.po
  • src/locales/sv/messages.mjs
  • src/locales/sv/messages.po
  • src/locales/tr/messages.mjs
  • src/locales/tr/messages.po
  • src/locales/uk/messages.mjs
  • src/locales/uk/messages.po
  • src/locales/zh-TW/messages.mjs
  • src/locales/zh-TW/messages.po
  • src/locales/zh/messages.mjs
  • src/locales/zh/messages.po
  • src/store/handlers/pushbot.ts
  • src/types/index.ts
  • tests/components/SlashParamHint.test.ts
  • tests/components/layout/ChatHeader.memberButton.test.tsx
  • tests/store/pushbot.test.ts
✅ Files skipped from review due to trivial changes (3)
  • src/locales/it/messages.mjs
  • src/locales/fr/messages.po
  • src/locales/ja/messages.po
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/irc/IRCClient.ts
  • src/components/layout/ChatArea.tsx

Comment on lines +33 to +69
const STATUS_BADGE: Record<
PushBotInfo["status"],
{ label: string; cls: string }
> = {
active: {
label: "active",
cls: "bg-emerald-700/40 text-emerald-300 border border-emerald-600/60",
},
pending: {
label: "pending",
cls: "bg-amber-700/40 text-amber-300 border border-amber-600/60",
},
suspended: {
label: "suspended",
cls: "bg-red-700/40 text-red-300 border border-red-600/60",
},
deleted: {
label: "deleted",
cls: "bg-discord-dark-200 text-discord-text-muted border border-discord-dark-500",
},
};

const SCOPE_BADGE: Record<
PushBotInfo["scope"],
{ label: string; title: string; cls: string }
> = {
server: {
label: "server-bot",
title: "Reachable from any channel",
cls: "bg-discord-primary/30 text-discord-primary border border-discord-primary/40",
},
channel: {
label: "channel-bot",
title: "Active only in joined channels",
cls: "bg-amber-700/30 text-amber-300 border border-amber-600/50",
},
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Localize badge labels/titles instead of hardcoding English constants.

Lines 33-69 define user-visible badge text/tooltips as raw literals (active, pending, server-bot, etc.), so they bypass translation extraction and remain English in localized builds.

As per coding guidelines, src/**/*.tsx: "Wrap JSX text children with <Trans>…</Trans> macro ... Use useLingui() with t template literal for JSX props..." and src/**/*.{ts,tsx}: "Use t template literal ... but never at module scope."

🤖 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 33 - 69, STATUS_BADGE and
SCOPE_BADGE currently hold user-facing hardcoded English strings at module
scope; move their creation into component/runtime scope and localize using
lingui's runtime API: inside the component that renders these badges call
useLingui() (or use the t`` template) to produce localized label/title strings
for the entries keyed by PushBotInfo["status"] and PushBotInfo["scope"], or
return them from helper functions (e.g., getStatusBadge(status) /
getScopeBadge(scope)) so no t`` calls are at module scope and JSX props receive
localized strings instead of raw literals.

Comment on lines +80 to +110
function badgeStyle(source: SlashSuggestionSource): BadgeStyle {
switch (source.kind) {
case "client":
return {
label: "client",
title: "Handled by ObsidianIRC before being sent",
className:
"bg-discord-dark-200 text-discord-text-muted border border-discord-dark-500",
};
case "server":
return {
label: "server",
title: "Command provided by the IRC server",
className:
"bg-emerald-700/40 text-emerald-300 border border-emerald-600/60",
};
case "bot":
return source.scope === "server"
? {
label: "server-bot",
title: "Server-wide bot — reachable from any channel",
className:
"bg-discord-primary/30 text-discord-primary border border-discord-primary/40",
}
: {
label: "channel-bot",
title: "Channel bot — present in this channel",
className:
"bg-amber-700/30 text-amber-300 border border-amber-600/50",
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make new source badges and header text translatable.

Lines 80-110 and 203-205 introduce user-visible strings (client, server, channel-bot, tooltip titles, Slash commands) as plain literals. These should be localized via Lingui (<Trans> / t) to avoid English-only UI in non-English locales.

As per coding guidelines, src/**/*.tsx: "Wrap JSX text children with <Trans>…</Trans> macro ... Use useLingui() with t template literal for JSX props like title=..."

Also applies to: 203-205

🤖 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/SlashCommandPopover.tsx` around lines 80 - 110, The
badgeStyle function returns user-visible labels and tooltip titles as hard-coded
strings; update badgeStyle to use Lingui's t template literal (via useLingui())
for all label and title values (e.g., replace "client", "server", "channel-bot",
and their title strings with t`...`) and ensure any JSX header text (the "Slash
commands" header rendered elsewhere around lines ~203-205) is wrapped with
<Trans>…</Trans>; import/use useLingui() to get t for prop strings (title=) and
wrap visible JSX children with <Trans>. Reference: badgeStyle (function) and the
component header rendering of "Slash commands". Ensure no plain English literals
remain for these badges or header.

Comment thread src/components/ui/SlashParamHint.tsx Outdated
Comment on lines +108 to +133
<span className="text-xs text-discord-text-muted ml-1">
{entry.source === "bot"
? `via @${entry.botNick}`
: "(handled by ObsidianIRC)"}
</span>
</div>
{opts[ctx.argIndex] && (
<div className="mt-1 text-xs text-discord-text-muted">
<span className="text-discord-text-normal font-medium">
{opts[ctx.argIndex].name}
</span>
{" — "}
<span>{opts[ctx.argIndex].type || "string"}</span>
{opts[ctx.argIndex].required && (
<span className="text-discord-red ml-1">required</span>
)}
{opts[ctx.argIndex].description && (
<span className="ml-1">— {opts[ctx.argIndex].description}</span>
)}
</div>
)}
{/* show choices if present */}
{opts[ctx.argIndex]?.choices &&
opts[ctx.argIndex].choices!.length > 0 && (
<div className="mt-1 text-xs text-discord-text-muted">
one of:{" "}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Localize new hint copy instead of shipping raw English strings.

Lines 108-133 introduce user-visible literals ((handled by ObsidianIRC), required, one of:) outside Lingui macros, which breaks localization for this new UI.

As per coding guidelines, src/**/*.tsx: "Wrap JSX text children with <Trans>…</Trans> macro ... Use useLingui() with t template literal for JSX props..."

🧰 Tools
🪛 GitHub Check: Lint and Formating

[warning] 131-131: lint/style/noNonNullAssertion
Forbidden non-null assertion.

🤖 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/SlashParamHint.tsx` around lines 108 - 133, Replace the raw
English literals in SlashParamHint.tsx with Lingui macros: wrap JSX text
children like "(handled by ObsidianIRC)", "required" and "one of:" in
<Trans>…</Trans> and use useLingui()/t`` for any prop/inline template strings
(e.g., the `via @${entry.botNick}` interpolation) so they are localizable;
update imports to include Trans and useLingui/t and adjust the JSX around
entry.source, opts[ctx.argIndex], and opts[ctx.argIndex].choices to use the
localized strings instead of hardcoded English.

Comment thread src/components/ui/SlashParamHint.tsx Outdated
Comment thread src/lib/clientCommands.ts
Comment on lines +27 to +114
export const CLIENT_COMMANDS: ClientCommand[] = [
{
name: "me",
description: "Send an action / emote",
options: [
{
name: "action",
type: "string",
required: true,
description: "What you're doing",
},
],
},
{
name: "msg",
description: "Open a private message to a user",
options: [
{ name: "user", type: "user", required: true },
{
name: "message",
type: "string",
required: true,
description: "First message to send",
},
],
},
{
name: "whisper",
description: "Whisper to a user in the current channel context",
scope: "channel-only",
options: [
{ name: "user", type: "user", required: true },
{ name: "message", type: "string", required: true },
],
},
{
name: "join",
description: "Join a channel",
options: [
{
name: "channel",
type: "channel",
required: true,
description: "Channel to join (#name)",
},
],
},
{
name: "part",
description: "Leave a channel",
options: [
{
name: "channel",
type: "channel",
required: false,
description: "Channel to leave (defaults to current)",
},
],
},
{
name: "nick",
description: "Change your nickname on this server",
options: [
{
name: "newnick",
type: "string",
required: true,
description: "New nickname",
},
],
},
{
name: "away",
description: "Mark yourself as away",
options: [
{
name: "reason",
type: "string",
required: false,
description: "Away message",
},
],
},
{
name: "back",
description: "Mark yourself as back",
},
];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Client command descriptions need an i18n-safe construction path.

Lines 27-114 keep user-visible descriptions in a module-scope constant, so they won’t follow the required translation pattern. Consider exposing a factory (called inside function scope) that returns localized command metadata.

As per coding guidelines, src/**/*.{ts,tsx}: "Use t template literal from @lingui/macro for simple strings outside JSX ... but never at module scope."

🤖 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/clientCommands.ts` around lines 27 - 114, The CLIENT_COMMANDS
constant holds user-visible descriptions at module scope and must be produced by
a factory called at runtime so localization via the t template literal is used
inside function scope; create a function (e.g., getClientCommands or
buildClientCommands) that imports t from '`@lingui/macro`' and returns the array
previously in CLIENT_COMMANDS, replacing every description and
option.description with t`...` calls, then update call sites to use
getClientCommands() and remove the module-scope CLIENT_COMMANDS constant to
ensure i18n-safe construction for commands like "me", "msg", "whisper", "join",
"part", "nick", "away", and "back".

Comment on lines +25 to +28
#: src/components/ui/BotsModal.tsx
msgid "(suspended)"
msgstr ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fill the new German translations instead of leaving msgstr "".

Several newly added bot-related strings are untranslated (msgstr ""), which will fail i18n checks and ship fallback text in German UI. Please populate these entries before merge and re-run extract/compile.

As per coding guidelines, src/locales/**/*.po: “When adding new user-visible strings, translate them before committing … then fill in all empty msgstr "" entries across every non-English .po file before committing”.

Also applies to: 240-243, 289-292, 414-417, 418-421, 422-425, 660-663, 664-667, 789-793, 1045-1048, 1049-1052, 1588-1591, 1711-1714, 1800-1803, 2105-2108, 2158-2161, 2174-2177, 2263-2266, 2363-2366, 2383-2386, 2414-2417, 2518-2521, 2556-2559, 2746-2749

🤖 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/de/messages.po` around lines 25 - 28, Populate the empty German
translations in src/locales/de/messages.po for all newly added bot-related msgid
entries (e.g., msgid "(suspended)" from src/components/ui/BotsModal.tsx and the
other listed untranslated msgids) by replacing each msgstr "" with the correct
German text, ensure plural forms if any are handled, save the file, then re-run
the i18n extract/compile step to validate no untranslated entries remain; focus
on the entries referenced in the review (the multiple blocks of untranslated bot
strings) and verify compilation succeeds.

Comment thread src/locales/es/messages.po
Comment on lines +25 to +27
#: src/components/ui/BotsModal.tsx
msgid "(suspended)"
msgstr ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Complete Finnish translations for the new bot-management strings.

There are newly added entries with msgstr "" (starting at Line 25 and repeated for many new bot-related keys), so the Finnish locale is incomplete for this feature.

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".

🤖 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 - 27, The Finnish locale has
empty translations for new bot-management strings (e.g., msgid "(suspended)"
used by BotsModal.tsx); run the extraction task (npm run i18n:extract) to list
all missing keys, then fill every empty msgstr "" for those bot-related msgid
entries in the fi/messages.po (including "(suspended)" and the other new bot
keys) so all non-English .po files contain translations before committing.

Comment on lines +26 to +27
msgid "(suspended)"
msgstr ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fill in all Italian translations before committing.

At least 23 new bot-related message entries have been added with empty msgstr "" values. Italian-speaking users will see English text instead of translated UI strings for the entire bot management feature.

Run npm run i18n:extract to identify all missing translations, then translate each empty msgstr "" entry:

  • (suspended) (line 27)
  • All (line 242)
  • Approve (line 291)
  • Bot hasn't registered any slash commands yet. (line 416)
  • Bots (line 420)
  • Bots on this network (line 424)
  • config-defined (line 661)
  • Config-defined bot. Edit obbyircd.conf and /REHASH to change state. (line 665)
  • offline (line 712)
  • Delete bot {0}? This soft-deletes... (line 791)
  • Gateway connected (line 1047)
  • gateway online (line 1051)
  • No bots registered on this network yet. (line 1589)
  • Operator actions (line 1801)
  • Search by nick or description (line 2106)
  • Select a bot on the left to see its commands and management actions. (line 2159)
  • self-registered (line 2175)
  • Server-wide (line 2264)
  • Slash commands (line 2364)
  • Source (line 2384)
  • Suspend (line 2415)
  • Unsuspend (line 2557)
  • Webhook (line 2747)

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: 241-242, 290-291, 415-416, 419-420, 423-424, 660-662, 664-666, 711-713, 790-792, 1046-1047, 1050-1051, 1588-1590, 1800-1802, 2105-2107, 2158-2160, 2174-2176, 2263-2265, 2363-2365, 2383-2385, 2414-2416, 2556-2558, 2746-2748

🤖 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 26 - 27, The Italian locale file has
many untranslated entries; run `npm run i18n:extract` to refresh missing keys
and then fill every empty msgstr "" in src/locales/it/messages.po for the listed
msgids (e.g. "(suspended)", "All", "Approve", "Bot hasn't registered any slash
commands yet.", "Bots", "Bots on this network", "config-defined",
"Config-defined bot. Edit obbyircd.conf and /REHASH to change state.",
"offline", the "Delete bot {0}?  This soft-deletes..." string, "Gateway
connected", "gateway online", "No bots registered on this network yet.",
"Operator actions", "Search by nick or description", "Select a bot on the left
to see its commands and management actions.", "self-registered", "Server-wide",
"Slash commands", "Source", "Suspend", "Unsuspend", "Webhook") providing proper
Italian translations for each msgid so no UI strings remain in English before
committing.

Comment thread src/locales/ko/messages.po
@ValwareIRC ValwareIRC force-pushed the feat/pushbot-client branch from 4de4db6 to ed750fc Compare May 16, 2026 21:11
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.
@ValwareIRC ValwareIRC force-pushed the feat/pushbot-client branch from ed750fc to f81cb89 Compare May 16, 2026 21:12
…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.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/locales/fi/messages.po (1)

25-27: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Complete Finnish translations for all newly added bot-management strings.

These new user-facing entries are still committed with empty msgstr "", so Finnish users will see fallback/mixed-language UI in the new Bots flows.

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: 240-242, 289-291, 416-418, 420-430, 669-675, 798-801, 1054-1060, 1597-1599, 1720-1722, 1809-1811, 2114-2116, 2167-2169, 2183-2185, 2272-2274, 2372-2374, 2392-2394, 2423-2425, 2527-2529, 2565-2567, 2755-2757

🤖 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 - 27, Several new bot-management
strings in the Finnish PO file are left untranslated (empty msgstr ""), causing
mixed-language UI; open src/locales/fi/messages.po and fill the msgstr for each
msgid listed (e.g. "(suspended)" and the other msgids referenced from
src/components/ui/BotsModal.tsx and the additional line groups in the review)
with accurate Finnish translations, then run npm run i18n:extract to ensure no
missing strings remain and commit the updated .po files so all non-English
locales include translations for the newly added user-visible strings.
src/locales/ko/messages.po (1)

25-27: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Fill all newly added Korean msgstr values before merge.

The new Bots-related entries are still untranslated (msgstr ""), which will ship a mixed-language Korean experience in this feature path.

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: 240-242, 289-291, 416-418, 420-430, 669-675, 798-801, 1054-1060, 1597-1599, 1720-1722, 1809-1811, 2114-2116, 2167-2169, 2183-2185, 2272-2274, 2372-2374, 2392-2394, 2423-2425, 2527-2529, 2565-2567, 2755-2757

🤖 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/ko/messages.po` around lines 25 - 27, The Korean .po file
contains untranslated entries (msgstr ""), e.g. the Bots-related msgid
"(suspended)" (from src/components/ui/BotsModal.tsx) and many other entries; run
the i18n extraction and fill all empty msgstr values before merging: run `npm
run i18n:extract` to get the current missing keys, then populate each empty
msgstr in src/locales/ko/messages.po (including the ranges you were called out
for) with proper Korean translations matching their msgid context (e.g., the
BotsModal strings), save the file and re-run the extraction/validation to ensure
no msgstr "" remain.
🤖 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.

Duplicate comments:
In `@src/locales/fi/messages.po`:
- Around line 25-27: Several new bot-management strings in the Finnish PO file
are left untranslated (empty msgstr ""), causing mixed-language UI; open
src/locales/fi/messages.po and fill the msgstr for each msgid listed (e.g.
"(suspended)" and the other msgids referenced from
src/components/ui/BotsModal.tsx and the additional line groups in the review)
with accurate Finnish translations, then run npm run i18n:extract to ensure no
missing strings remain and commit the updated .po files so all non-English
locales include translations for the newly added user-visible strings.

In `@src/locales/ko/messages.po`:
- Around line 25-27: The Korean .po file contains untranslated entries (msgstr
""), e.g. the Bots-related msgid "(suspended)" (from
src/components/ui/BotsModal.tsx) and many other entries; run the i18n extraction
and fill all empty msgstr values before merging: run `npm run i18n:extract` to
get the current missing keys, then populate each empty msgstr in
src/locales/ko/messages.po (including the ranges you were called out for) with
proper Korean translations matching their msgid context (e.g., the BotsModal
strings), save the file and re-run the extraction/validation to ensure no msgstr
"" remain.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6e715028-3072-4679-a54a-b55f38e17f75

📥 Commits

Reviewing files that changed from the base of the PR and between 4de4db6 and a19dfd9.

📒 Files selected for processing (47)
  • src/components/layout/ChatArea.tsx
  • src/components/layout/ChatHeader.tsx
  • src/components/ui/BotsModal.tsx
  • src/components/ui/SlashParamHint.tsx
  • src/lib/irc/IRCClient.ts
  • src/locales/cs/messages.mjs
  • src/locales/cs/messages.po
  • src/locales/de/messages.mjs
  • src/locales/de/messages.po
  • src/locales/en/messages.mjs
  • src/locales/en/messages.po
  • src/locales/es/messages.mjs
  • src/locales/es/messages.po
  • src/locales/fi/messages.mjs
  • src/locales/fi/messages.po
  • src/locales/fr/messages.mjs
  • src/locales/fr/messages.po
  • src/locales/it/messages.mjs
  • src/locales/it/messages.po
  • src/locales/ja/messages.mjs
  • src/locales/ja/messages.po
  • src/locales/ko/messages.mjs
  • src/locales/ko/messages.po
  • src/locales/nl/messages.mjs
  • src/locales/nl/messages.po
  • src/locales/pl/messages.mjs
  • src/locales/pl/messages.po
  • src/locales/pt/messages.mjs
  • src/locales/pt/messages.po
  • src/locales/ro/messages.mjs
  • src/locales/ro/messages.po
  • src/locales/ru/messages.mjs
  • src/locales/ru/messages.po
  • src/locales/sv/messages.mjs
  • src/locales/sv/messages.po
  • src/locales/tr/messages.mjs
  • src/locales/tr/messages.po
  • src/locales/uk/messages.mjs
  • src/locales/uk/messages.po
  • src/locales/zh-TW/messages.mjs
  • src/locales/zh-TW/messages.po
  • src/locales/zh/messages.mjs
  • src/locales/zh/messages.po
  • src/store/handlers/pushbot.ts
  • src/types/index.ts
  • tests/components/layout/ChatHeader.memberButton.test.tsx
  • tests/store/pushbot.test.ts
✅ Files skipped from review due to trivial changes (8)
  • src/locales/fr/messages.mjs
  • src/locales/ko/messages.mjs
  • src/locales/it/messages.mjs
  • src/locales/fi/messages.mjs
  • src/locales/de/messages.mjs
  • src/locales/fr/messages.po
  • src/locales/cs/messages.po
  • src/locales/it/messages.po
🚧 Files skipped from review as they are similar to previous changes (10)
  • src/lib/irc/IRCClient.ts
  • src/locales/en/messages.mjs
  • src/locales/es/messages.mjs
  • src/locales/ja/messages.mjs
  • src/components/ui/SlashParamHint.tsx
  • src/components/layout/ChatHeader.tsx
  • src/components/layout/ChatArea.tsx
  • src/locales/cs/messages.mjs
  • src/components/ui/BotsModal.tsx
  • src/locales/ja/messages.po

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.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

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-2912: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Regenerate and commit the i18n catalog artifacts.

CI is currently failing the i18n catalog check, which indicates this file is not fully synchronized with extraction/compile output. Please run npm run i18n:extract && npm run i18n:compile and commit the resulting src/locales/ updates.

🤖 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 - 2912, The i18n catalog in
src/locales/en/messages.po is out-of-sync causing CI to fail; run the extraction
and compile scripts (npm run i18n:extract && npm run i18n:compile) to regenerate
locale artifacts, review the changes under src/locales/, add and commit the
updated files (including src/locales/en/messages.po and any compiled JSON), and
push the commit so CI sees the synchronized i18n catalog.
♻️ Duplicate comments (3)
src/hooks/useMessageSending.ts (2)

120-120: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use UTF-8-safe base64 encoding for bot payloads.

btoa(JSON.stringify(payload)) can throw on valid Unicode input.

Proposed fix
   const payload = { name: cmd.name, options };
-  const b64 = btoa(JSON.stringify(payload)).replace(/=+$/, ""); // strip trailing padding for IRCv3 tag-value friendliness
+  const json = JSON.stringify(payload);
+  const bytes = new TextEncoder().encode(json);
+  const bin = Array.from(bytes, (b) => String.fromCharCode(b)).join("");
+  const b64 = btoa(bin).replace(/=+$/, ""); // strip trailing padding for IRCv3 tag-value friendliness
Does btoa throw for non-ASCII characters, and what is the recommended UTF-8-safe encoding approach?
🤖 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/hooks/useMessageSending.ts` at line 120, Replace the unsafe
btoa(JSON.stringify(payload)) call with a UTF-8-safe base64 encoding: serialize
payload to a string, encode it to UTF-8 bytes (use
TextEncoder().encode(serialized)), convert the bytes to a binary string via
String.fromCharCode(...bytes) and then call btoa(...) and strip padding as
before; update the b64 assignment in useMessageSending.ts (the b64 variable that
currently uses btoa and the payload serialization) and add a small
fallback/polyfill note if TextEncoder is unavailable in the runtime.

96-96: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject invalid integer args before adding them to payload.

parseInt can return NaN, and JSON.stringify converts it to null, which silently corrupts command options.

Proposed fix
-    if (o.type === "int") options[o.name] = Number.parseInt(raw, 10);
+    if (o.type === "int") {
+      const parsed = Number.parseInt(raw, 10);
+      if (Number.isNaN(parsed)) return false;
+      options[o.name] = parsed;
+    }
In JavaScript, what does JSON.stringify({x: NaN}) produce?
🤖 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/hooks/useMessageSending.ts` at line 96, The code in useMessageSending.ts
currently does options[o.name] = Number.parseInt(raw, 10) when o.type === "int",
which allows NaN to be inserted and later serialized as null; change this to
validate the parsed integer before setting the option: parse with
Number.parseInt(raw, 10) (or Number(raw) if appropriate), check
Number.isInteger(parsed) (or !Number.isNaN(parsed)) and if invalid either
reject/throw an error or skip/return a validation error to the caller instead of
assigning it to options[o.name]; update the logic around the o.type === "int"
branch and any callers that expect a value so invalid integers are rejected
before being added to the payload.
src/locales/es/messages.po (1)

25-27: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Complete Spanish translations for new bot/slash-command strings before merge.

Many newly added user-visible entries are still untranslated (msgstr ""), e.g., Line 27, Line 257, Line 306, Line 433, Line 440, Line 445, and multiple later bot-related entries. This will leak English text into the Spanish locale.

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-433, 435-446, 685-687, 689-691, 814-817, 1070-1072, 1074-1076, 1613-1615, 1736-1738, 1825-1827, 2138-2140, 2191-2193, 2207-2209, 2296-2299, 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/es/messages.po` around lines 25 - 27, The Spanish .po file
contains empty translations (e.g., msgid "(suspended)" from
src/components/ui/BotsModal.tsx) and many other bot/slash-command msgstr ""
entries; run npm run i18n:extract to get the full list, then provide Spanish
translations for every msgid with an empty msgstr in this file (including the
groups called out around the bot-related entries) so no English text is left
untranslated before merging—update each msgstr "" to the correct Spanish string
matching its msgid and context.
🤖 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-2416: The guard that skips channel-scoped bots only checks
for when selectedChannel exists, so in DMs channel-scoped bot commands still get
included; update the condition in ChatArea (where botScope, selectedChannel,
chanUsers, scope are used) to skip when botScope === "channel" AND either there
is no selectedChannel OR the bot is not in the channel (i.e. change the check to
if (botScope === "channel" && (!selectedChannel || !inChannel)) continue; and
make the same change at the other occurrence around the scope assignment (the
block at lines ~2544-2548).

In `@src/components/message/BotInvocationChip.tsx`:
- Around line 24-26: The Base64 decoding in BotInvocationChip.tsx currently uses
atob(padded) and passes that directly to JSON.parse which corrupts
non-ASCII/UTF-8 payloads; replace the atob->JSON.parse flow by converting the
atob result into a Uint8Array (map each char to charCodeAt) and then decode with
a TextDecoder('utf-8') to get a proper UTF-8 string, then JSON.parse that string
(update uses of variables padded, json, obj accordingly and keep error handling
around JSON.parse).

In `@src/components/ui/SlashCommandParamModal.tsx`:
- Line 368: The UI has hardcoded user-facing strings in the
SlashCommandParamModal component (e.g., the input placeholder "nick", the
`required` label and the close button icon semantics), so update
SlashCommandParamModal to localize them: import { Trans } from "`@lingui/macro`"
and use <Trans>…</Trans> for any JSX text children, and import useLingui/use the
t`...` template for JSX props (placeholder=, aria-label=, title=) — replace
placeholder="nick" with placeholder={t`nick`}, replace raw "required" text nodes
with <Trans>required</Trans> (or equivalent), and add an accessible label to the
close button using aria-label={t`Close`} (and title={t`Close`} if present);
apply the same changes for the other occurrences noted around the component (the
elements at the other two spots referenced in the comment).

In `@src/hooks/useMessageSending.ts`:
- Around line 77-82: The fallback branch that populates matches currently
considers any bot in the bots map; restrict it to server-scoped bots only by
skipping entries that are channel-scoped. Update the loop in useMessageSending
(the block that iterates Object.entries(bots) and uses lowerCmd to find a cmd)
to only examine bots/command lists that indicate server scope (e.g., check
bot.scope === 'server' or cmd.scope === 'server' depending on where scope is
stored) before running list.find and pushing to matches, so channel-scoped bots
in other channels are ignored.

In `@src/lib/countries.ts`:
- Around line 14-16: The current check only enforces length for the `code`
parameter and allows invalid inputs like "1_" to produce bad glyphs; update the
validation to require two ASCII letters (A–Z) before computing code points.
Replace the length check with a pattern test (e.g. /^[A-Za-z]{2}$/) or
explicitly verify both characters are letters, then call `toUpperCase()` and use
the existing `cp` helper and String.fromCodePoint logic unchanged (i.e.,
validate `code` first, then compute using `cp(code[0])` and `cp(code[1])`).

In `@src/locales/fr/messages.po`:
- Around line 25-27: The FR locale file has many untranslated entries (empty
msgstr) including the new bot-related msgid "(suspended)" from
src/components/ui/BotsModal.tsx; update that msgstr with the correct French
translation and scan the same file to fill all other empty msgstr occurrences
mentioned (lines ranges listed in the review: e.g., entries around 255-257,
304-306, 431-446, 685-692, etc.), ensuring each msgid is translated into French
and preserving punctuation/casing so non-English users see localized text.

In `@src/locales/ja/messages.po`:
- Around line 25-27: The file src/locales/ja/messages.po contains many msgid
entries (e.g., msgid "(suspended)") with empty msgstr values; fill each empty
msgstr with the correct Japanese translation for the corresponding msgid
(including the ranges noted such as lines near msgids 255-257, 304-306, 431-446,
etc.), ensuring every non-English .po entry in this file is translated before
merging; preserve msgid text, keep plural forms if present, and follow existing
translation style/terminology used elsewhere in src/locales/ja to maintain
consistency.

---

Outside diff comments:
In `@src/locales/en/messages.po`:
- Around line 1-2912: The i18n catalog in src/locales/en/messages.po is
out-of-sync causing CI to fail; run the extraction and compile scripts (npm run
i18n:extract && npm run i18n:compile) to regenerate locale artifacts, review the
changes under src/locales/, add and commit the updated files (including
src/locales/en/messages.po and any compiled JSON), and push the commit so CI
sees the synchronized i18n catalog.

---

Duplicate comments:
In `@src/hooks/useMessageSending.ts`:
- Line 120: Replace the unsafe btoa(JSON.stringify(payload)) call with a
UTF-8-safe base64 encoding: serialize payload to a string, encode it to UTF-8
bytes (use TextEncoder().encode(serialized)), convert the bytes to a binary
string via String.fromCharCode(...bytes) and then call btoa(...) and strip
padding as before; update the b64 assignment in useMessageSending.ts (the b64
variable that currently uses btoa and the payload serialization) and add a small
fallback/polyfill note if TextEncoder is unavailable in the runtime.
- Line 96: The code in useMessageSending.ts currently does options[o.name] =
Number.parseInt(raw, 10) when o.type === "int", which allows NaN to be inserted
and later serialized as null; change this to validate the parsed integer before
setting the option: parse with Number.parseInt(raw, 10) (or Number(raw) if
appropriate), check Number.isInteger(parsed) (or !Number.isNaN(parsed)) and if
invalid either reject/throw an error or skip/return a validation error to the
caller instead of assigning it to options[o.name]; update the logic around the
o.type === "int" branch and any callers that expect a value so invalid integers
are rejected before being added to the payload.

In `@src/locales/es/messages.po`:
- Around line 25-27: The Spanish .po file contains empty translations (e.g.,
msgid "(suspended)" from src/components/ui/BotsModal.tsx) and many other
bot/slash-command msgstr "" entries; run npm run i18n:extract to get the full
list, then provide Spanish translations for every msgid with an empty msgstr in
this file (including the groups called out around the bot-related entries) so no
English text is left untranslated before merging—update each msgstr "" to the
correct Spanish string matching its msgid and context.
🪄 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: 28ebfb9d-fff6-4464-8c74-85fe41da2bc8

📥 Commits

Reviewing files that changed from the base of the PR and between a19dfd9 and 7b6aa8b.

📒 Files selected for processing (48)
  • src/components/layout/ChatArea.tsx
  • src/components/message/BotInvocationChip.tsx
  • src/components/message/MessageItem.tsx
  • src/components/ui/SlashCommandParamModal.tsx
  • src/components/ui/SlashCommandPopover.tsx
  • src/components/ui/SlashParamHint.tsx
  • src/hooks/useMessageSending.ts
  • src/lib/countries.ts
  • src/locales/cs/messages.mjs
  • src/locales/cs/messages.po
  • src/locales/de/messages.mjs
  • src/locales/de/messages.po
  • src/locales/en/messages.mjs
  • src/locales/en/messages.po
  • src/locales/es/messages.mjs
  • src/locales/es/messages.po
  • src/locales/fi/messages.mjs
  • src/locales/fi/messages.po
  • src/locales/fr/messages.mjs
  • src/locales/fr/messages.po
  • src/locales/it/messages.mjs
  • src/locales/it/messages.po
  • src/locales/ja/messages.mjs
  • src/locales/ja/messages.po
  • src/locales/ko/messages.mjs
  • src/locales/ko/messages.po
  • src/locales/nl/messages.mjs
  • src/locales/nl/messages.po
  • src/locales/pl/messages.mjs
  • src/locales/pl/messages.po
  • src/locales/pt/messages.mjs
  • src/locales/pt/messages.po
  • src/locales/ro/messages.mjs
  • src/locales/ro/messages.po
  • src/locales/ru/messages.mjs
  • src/locales/ru/messages.po
  • src/locales/sv/messages.mjs
  • src/locales/sv/messages.po
  • src/locales/tr/messages.mjs
  • src/locales/tr/messages.po
  • src/locales/uk/messages.mjs
  • src/locales/uk/messages.po
  • src/locales/zh-TW/messages.mjs
  • src/locales/zh-TW/messages.po
  • src/locales/zh/messages.mjs
  • src/locales/zh/messages.po
  • src/types/index.ts
  • tests/components/SlashParamHint.test.ts
✅ Files skipped from review due to trivial changes (4)
  • src/locales/ko/messages.mjs
  • src/locales/fr/messages.mjs
  • src/locales/it/messages.mjs
  • src/locales/de/messages.mjs
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/locales/cs/messages.mjs
  • src/locales/en/messages.mjs
  • src/locales/fi/messages.mjs
  • src/locales/ja/messages.mjs
  • src/locales/es/messages.mjs
  • src/components/ui/SlashParamHint.tsx
  • src/locales/it/messages.po

Comment on lines +2411 to +2416
const botScope: "channel" | "server" =
srv.bots?.[botNick]?.scope ?? "channel";
const inChannel = chanUsers.has(botNick);
if (botScope === "channel" && selectedChannel && !inChannel)
continue;
const scope: "channel" | "server" = botScope;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Exclude channel-scoped bot commands outside channel context.

Current guard only applies when selectedChannel exists. In DM views, channel-scoped bot commands are still included, which creates invalid/noisy suggestions.

Proposed fix
-                    if (botScope === "channel" && selectedChannel && !inChannel)
-                      continue;
+                    if (botScope === "channel" && !selectedChannel) continue;
+                    if (botScope === "channel" && !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 - 2416, The guard that
skips channel-scoped bots only checks for when selectedChannel exists, so in DMs
channel-scoped bot commands still get included; update the condition in ChatArea
(where botScope, selectedChannel, chanUsers, scope are used) to skip when
botScope === "channel" AND either there is no selectedChannel OR the bot is not
in the channel (i.e. change the check to if (botScope === "channel" &&
(!selectedChannel || !inChannel)) continue; and make the same change at the
other occurrence around the scope assignment (the block at lines ~2544-2548).

Comment on lines +24 to +26
const json = atob(padded);
const obj = JSON.parse(json);
if (!obj || typeof obj !== "object") return null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does using atob(...) output directly with JSON.parse(...) preserve Unicode correctly, or should TextDecoder be used on bytes?

💡 Result:

You should not use atob(...) output directly with JSON.parse(...) if your data contains Unicode characters [1][2][3]. The atob function is designed to decode Base64 data into a binary string where each character represents a single byte (0–255) [4][1]. Because JavaScript strings are UTF-16 encoded, atob treats these bytes as individual characters rather than interpreting them as UTF-8 encoded text [5][1]. If your Base64 data contains multi-byte UTF-8 characters (such as emojis, accented letters, or non-Latin scripts), passing the output of atob directly to JSON.parse will result in corrupted text or decoding errors [6][2]. To correctly handle Unicode, you must convert the binary string output from atob into a byte array (Uint8Array) and then use TextDecoder to interpret those bytes as UTF-8 [1][2][3]. Recommended approach in browsers: 1. Decode the Base64 string using atob. 2. Convert the resulting binary string into a Uint8Array [2][3]. 3. Use TextDecoder to decode the Uint8Array into a valid UTF-8 string [1][2]. 4. Pass the resulting string to JSON.parse [2]. Example: const binaryString = atob(base64Data); const bytes = Uint8Array.from(binaryString, (m) => m.codePointAt(0)); const decodedString = new TextDecoder.decode(bytes); const json = JSON.parse(decodedString); In modern environments, you can also use Uint8Array.fromBase64(base64Data) to directly obtain the byte array, bypassing the need for atob entirely [4][2]. In Node.js, you can use Buffer.from(base64Data, 'base64').toString('utf-8') to handle this process natively [1][2].

Citations:


🏁 Script executed:

cat -n src/components/message/BotInvocationChip.tsx | head -50

Repository: ObsidianIRC/ObsidianIRC

Length of output: 2003


🏁 Script executed:

rg -i "BotInvocationChip\|invoked-by" --type ts --type tsx -A 3 -B 3

Repository: ObsidianIRC/ObsidianIRC

Length of output: 95


🏁 Script executed:

rg "BotInvocationChip|invoked-by" -A 3 -B 3

Repository: ObsidianIRC/ObsidianIRC

Length of output: 8620


🏁 Script executed:

find . -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "decode\|BotInvocationChip" 2>/dev/null || echo "No test files found with these keywords"

Repository: ObsidianIRC/ObsidianIRC

Length of output: 270


🏁 Script executed:

cat tests/components/MiniMediaPlayer.test.tsx | head -100

Repository: ObsidianIRC/ObsidianIRC

Length of output: 2835


🏁 Script executed:

rg "obby.world" -A 5 -B 5

Repository: ObsidianIRC/ObsidianIRC

Length of output: 7148


🏁 Script executed:

rg "decodeBotInfo" -A 15 -B 2

Repository: ObsidianIRC/ObsidianIRC

Length of output: 2659


🏁 Script executed:

rg "decodeB64Json" -A 15 -B 2

Repository: ObsidianIRC/ObsidianIRC

Length of output: 2661


Decode base64 JSON as UTF-8 bytes, not as a direct JS string.

atob() output is a Latin-1 binary string; passing it directly to JSON.parse() corrupts Unicode payloads (e.g., non-ASCII option values). Use Uint8Array + TextDecoder to properly decode UTF-8:

Proposed fix
-    const json = atob(padded);
-    const obj = JSON.parse(json);
+    const bin = atob(padded);
+    const bytes = Uint8Array.from(bin, (ch) => ch.charCodeAt(0));
+    const json = new TextDecoder().decode(bytes);
+    const obj = JSON.parse(json);
📝 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
const json = atob(padded);
const obj = JSON.parse(json);
if (!obj || typeof obj !== "object") return null;
const bin = atob(padded);
const bytes = Uint8Array.from(bin, (ch) => ch.charCodeAt(0));
const json = new TextDecoder().decode(bytes);
const obj = JSON.parse(json);
if (!obj || typeof obj !== "object") return null;
🤖 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/message/BotInvocationChip.tsx` around lines 24 - 26, The
Base64 decoding in BotInvocationChip.tsx currently uses atob(padded) and passes
that directly to JSON.parse which corrupts non-ASCII/UTF-8 payloads; replace the
atob->JSON.parse flow by converting the atob result into a Uint8Array (map each
char to charCodeAt) and then decode with a TextDecoder('utf-8') to get a proper
UTF-8 string, then JSON.parse that string (update uses of variables padded,
json, obj accordingly and keep error handling around JSON.parse).

value={String(value ?? "")}
onChange={(e) => setValue(e.target.value)}
className={BASE_INPUT}
placeholder="nick"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Localize hardcoded UI strings and add an accessible label for close button.

These new user-facing strings bypass Lingui (placeholder, required, close icon semantics), so they won’t be translatable/accessibly announced.

Proposed fix
 import { Trans, t } from "`@lingui/macro`";
 import type React from "react";
 import { useCallback, useEffect, useMemo, useRef, useState } from "react";
@@
 export const SlashCommandParamModal: React.FC<Props> = ({
@@
 }) => {
+  const { t: tt } = useLingui();
@@
           <button
             type="button"
             onClick={onClose}
             className="text-discord-text-muted hover:text-white ml-2"
+            aria-label={tt`Close`}
           >
             ✕
           </button>
@@
               placeholder="nick"
+              placeholder={tt`nick`}
@@
               placeholder="`#channel`"
+              placeholder={tt`#channel`}
@@
-          <span className="text-discord-red ml-1 text-xs">required</span>
+          <span className="text-discord-red ml-1 text-xs">
+            <Trans>required</Trans>
+          </span>

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: 391-391, 422-422

🤖 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` at line 368, The UI has
hardcoded user-facing strings in the SlashCommandParamModal component (e.g., the
input placeholder "nick", the `required` label and the close button icon
semantics), so update SlashCommandParamModal to localize them: import { Trans }
from "`@lingui/macro`" and use <Trans>…</Trans> for any JSX text children, and
import useLingui/use the t`...` template for JSX props (placeholder=,
aria-label=, title=) — replace placeholder="nick" with placeholder={t`nick`},
replace raw "required" text nodes with <Trans>required</Trans> (or equivalent),
and add an accessible label to the close button using aria-label={t`Close`} (and
title={t`Close`} if present); apply the same changes for the other occurrences
noted around the component (the elements at the other two spots referenced in
the comment).

Comment on lines +77 to +82
if (!matches.length) {
for (const [bot, list] of Object.entries(bots)) {
const cmd = list.find((c) => c.name.toLowerCase() === lowerCmd);
if (cmd) matches.push({ bot, cmd });
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Limit fallback matching to server-scoped bots only.

This branch currently picks any known bot defining the command, including channel-scoped bots outside the active channel, which contradicts the documented resolution order and can misroute invocations.

Proposed fix
   if (!matches.length) {
     for (const [bot, list] of Object.entries(bots)) {
+      const scope = server.bots?.[bot]?.scope ?? "channel";
+      if (scope !== "server") continue;
       const cmd = list.find((c) => c.name.toLowerCase() === lowerCmd);
       if (cmd) matches.push({ bot, cmd });
     }
   }
📝 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 (!matches.length) {
for (const [bot, list] of Object.entries(bots)) {
const cmd = list.find((c) => c.name.toLowerCase() === lowerCmd);
if (cmd) matches.push({ bot, cmd });
}
}
if (!matches.length) {
for (const [bot, list] of Object.entries(bots)) {
const scope = server.bots?.[bot]?.scope ?? "channel";
if (scope !== "server") continue;
const cmd = list.find((c) => c.name.toLowerCase() === lowerCmd);
if (cmd) matches.push({ bot, cmd });
}
}
🤖 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/hooks/useMessageSending.ts` around lines 77 - 82, The fallback branch
that populates matches currently considers any bot in the bots map; restrict it
to server-scoped bots only by skipping entries that are channel-scoped. Update
the loop in useMessageSending (the block that iterates Object.entries(bots) and
uses lowerCmd to find a cmd) to only examine bots/command lists that indicate
server scope (e.g., check bot.scope === 'server' or cmd.scope === 'server'
depending on where scope is stored) before running list.find and pushing to
matches, so channel-scoped bots in other channels are ignored.

Comment thread src/lib/countries.ts
Comment on lines +14 to +16
if (!code || code.length !== 2) return "";
const cp = (c: string) => 0x1f1e6 + (c.toUpperCase().charCodeAt(0) - 65);
return String.fromCodePoint(cp(code[0])) + String.fromCodePoint(cp(code[1]));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate ISO alpha-2 characters before generating flag code points.

Length-only validation accepts values like "1_", which yields invalid glyphs. Require A-Z letters explicitly.

Proposed fix
 export function flagEmoji(code: string): string {
-  if (!code || code.length !== 2) return "";
+  if (!code || !/^[A-Za-z]{2}$/.test(code)) return "";
   const cp = (c: string) => 0x1f1e6 + (c.toUpperCase().charCodeAt(0) - 65);
   return String.fromCodePoint(cp(code[0])) + String.fromCodePoint(cp(code[1]));
 }
📝 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 (!code || code.length !== 2) return "";
const cp = (c: string) => 0x1f1e6 + (c.toUpperCase().charCodeAt(0) - 65);
return String.fromCodePoint(cp(code[0])) + String.fromCodePoint(cp(code[1]));
export function flagEmoji(code: string): string {
if (!code || !/^[A-Za-z]{2}$/.test(code)) return "";
const cp = (c: string) => 0x1f1e6 + (c.toUpperCase().charCodeAt(0) - 65);
return String.fromCodePoint(cp(code[0])) + String.fromCodePoint(cp(code[1]));
}
🤖 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/countries.ts` around lines 14 - 16, The current check only enforces
length for the `code` parameter and allows invalid inputs like "1_" to produce
bad glyphs; update the validation to require two ASCII letters (A–Z) before
computing code points. Replace the length check with a pattern test (e.g.
/^[A-Za-z]{2}$/) or explicitly verify both characters are letters, then call
`toUpperCase()` and use the existing `cp` helper and String.fromCodePoint logic
unchanged (i.e., validate `code` first, then compute using `cp(code[0])` and
`cp(code[1])`).

Comment on lines +25 to +27
#: src/components/ui/BotsModal.tsx
msgid "(suspended)"
msgstr ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Translate newly added French bot-related strings before merge.

Many newly added user-visible entries still have msgstr "", so French users will see fallback English in these flows. Please fill the new empty translations in this file before landing.

As per coding guidelines, src/locales/**/*.po: “When adding new user-visible strings, translate them before committing … 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, 1613-1616, 1736-1738, 1825-1828, 2138-2140, 2191-2193, 2207-2209, 2296-2298, 2396-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/fr/messages.po` around lines 25 - 27, The FR locale file has many
untranslated entries (empty msgstr) including the new bot-related msgid
"(suspended)" from src/components/ui/BotsModal.tsx; update that msgstr with the
correct French translation and scan the same file to fill all other empty msgstr
occurrences mentioned (lines ranges listed in the review: e.g., entries around
255-257, 304-306, 431-446, 685-692, etc.), ensuring each msgid is translated
into French and preserving punctuation/casing so non-English users see localized
text.

Comment thread src/locales/ja/messages.po
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants