chore: repo infrastructure setup (Biome, CI, docs, Vitest)#16
chore: repo infrastructure setup (Biome, CI, docs, Vitest)#16BillChirico merged 36 commits intomainfrom
Conversation
- Add @biomejs/biome as dev dependency - Configure biome.json with ESM/Node.js defaults - Add lint, lint:fix, format, and test scripts to package.json
- Apply consistent formatting (single quotes, trailing commas, 2-space indent) - Add node: protocol to Node.js builtin imports - Fix unused imports and variables - Fix forEach callback return value issues
- ci.yml: lint (Biome) and test (Vitest) on push/PR to main - claude-review.yml: Claude Code PR review via anthropics/claude-code-action
- Overhaul README.md with badges, architecture, config reference, env vars - Add AGENTS.md for AI coding agent context - Add CLAUDE.md for Claude Code context - Add CONTRIBUTING.md with contribution guidelines - Add .editorconfig for consistent editor settings
- Add vitest as dev dependency - Create vitest.config.js for Node.js ESM environment - Add tests/config.test.js — validates config.json structure - Add tests/commands.test.js — validates command file exports - All 13 tests passing
- Support OPENCLAW_API_URL/OPENCLAW_API_KEY with legacy aliases - Add src/deploy-commands.js using DISCORD_CLIENT_ID (fallback CLIENT_ID) - Update .env.example with required variables and comments - Update README env reference to standardized names - Update logging/error guidance for new OpenClaw env vars
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughAdds editor/formatting configs, CI and automated review workflows, expanded documentation and env examples, package/tooling updates, logging and module refinements, a deploy-commands script, and a large suite of Vitest tests covering commands, modules, utils, DB, and startup logic. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Enabled Biome noConsole rule as error. Replaced 19 console.log/warn/error calls across 6 files with proper Winston logger imports.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
2 similar comments
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
✅ Unit tests committed locally. Commit: |
|
✅ Unit tests committed locally. Commit: |
|
✅ Created PR with unit tests: #17 |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/commands/status.js (1)
25-39:⚠️ Potential issue | 🟡 Minor
formatRelativeTimedoesn't handle future timestamps (negativediff).If
timestampis in the future (e.g., due to clock skew),diffbecomes negative. The function would fall through all guards and return something like"-1d ago"or"0s ago"depending on rounding.🛡️ Suggested guard
function formatRelativeTime(timestamp) { if (!timestamp) return 'Never'; const now = Date.now(); const diff = now - timestamp; + if (diff < 0) return 'Just now'; const seconds = Math.floor(diff / 1000);src/modules/events.js (1)
108-115: 🧹 Nitpick | 🔵 TrivialSplit messages are sent via
send()instead ofreply(), losing the reply context.When the response is short, line 114 uses
message.reply(response), which threads the response under the user's message. When the response is long and split, all chunks are sent viamessage.channel.send(chunk), losing the reply association. Consider usingreply()for the first chunk:♻️ Suggested improvement
if (needsSplitting(response)) { const chunks = splitMessage(response); - for (const chunk of chunks) { - await message.channel.send(chunk); + for (let i = 0; i < chunks.length; i++) { + if (i === 0) { + await message.reply(chunks[i]); + } else { + await message.channel.send(chunks[i]); + } }src/modules/ai.js (2)
100-112:⚠️ Potential issue | 🟠 Major
fetch()call has no timeout — can block indefinitely if the API hangs.The
fetchtoOPENCLAW_URLhas noAbortSignal.timeout()or similar mechanism. If the upstream API stalls, this will hold the calling context (and potentially a Discord message handler) open forever.⏱️ Proposed fix: add a request timeout
+ const REQUEST_TIMEOUT_MS = 30_000; + try { const response = await fetch(OPENCLAW_URL, { method: 'POST', headers: { 'Content-Type': 'application/json', ...(OPENCLAW_TOKEN && { Authorization: `Bearer ${OPENCLAW_TOKEN}` }), }, body: JSON.stringify({ model: config.ai?.model || 'claude-sonnet-4-20250514', max_tokens: config.ai?.maxTokens || 1024, messages: messages, }), + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), });
114-119:⚠️ Potential issue | 🟡 Minor
healthMonitor.setAPIStatus('error')is called twice for HTTP error responses.When
response.okis false:
- Line 116:
healthMonitor.setAPIStatus('error')is called inside theif (!response.ok)block.- Line 118:
throwsends control to thecatchblock.- Line 141:
healthMonitor.setAPIStatus('error')is called again.This is a minor redundancy but indicates a control-flow smell. Consider removing the call at Line 116 since the
catchblock already handles it, or restructure so thethrowpath doesn't also go through the catch's health update.♻️ Proposed fix: remove the duplicate health status call
if (!response.ok) { - if (healthMonitor) { - healthMonitor.setAPIStatus('error'); - } throw new Error(`API error: ${response.status} ${response.statusText}`); }src/utils/errors.js (1)
51-56:⚠️ Potential issue | 🟡 MinorPre-existing:
ETIMEDOUTis classified asNETWORKinstead ofTIMEOUT.
ETIMEDOUTmatches at Line 51 and returnsErrorType.NETWORK, making the duplicate check at Line 54 unreachable. This predates this PR's changes, but if you're touching this file, consider fixing the classification:♻️ Suggested fix
- if (code === 'ECONNREFUSED' || code === 'ENOTFOUND' || code === 'ETIMEDOUT') { + if (code === 'ECONNREFUSED' || code === 'ENOTFOUND') { return ErrorType.NETWORK; } - if (code === 'ETIMEDOUT' || message.includes('timeout')) { + if (code === 'ETIMEDOUT' || message.includes('timeout')) { return ErrorType.TIMEOUT; }src/index.js (2)
225-264:⚠️ Potential issue | 🟡 MinorNo re-entrancy guard on
gracefulShutdown.If the process receives both SIGTERM and SIGINT in quick succession (common in container orchestrators),
gracefulShutdownruns concurrently, potentially callingclient.destroy()andprocess.exit(0)twice. A simple boolean flag prevents this.🛡️ Proposed fix
+let isShuttingDown = false; + async function gracefulShutdown(signal) { + if (isShuttingDown) return; + isShuttingDown = true; info('Shutdown initiated', { signal });
149-159:⚠️ Potential issue | 🔴 CriticalChange
'clientReady'to'ready'for discord.js v14 compatibility.In discord.js v14, the correct event string is
'ready', not'clientReady'. This event handler will never fire with the current string, preventing slash command registration and the execution of the newprocess.env.DISCORD_TOKENcall on line 155. Additionally, line 20 insrc/modules/events.jshas the same issue.
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 23-24: The CI uses a fixed node-version: 22 but AGENTS.md claims
support for "Node.js 18+"; either align CI with the documented range or update
the docs: modify the GitHub Actions job that sets node-version to use a matrix
(replace the single node-version: 22 with a strategy.matrix setting including at
least "18" and "22") so tests run against the minimum supported version and the
current one, or if you intend to support only Node 22, update AGENTS.md to
explicitly state Node 22; locate the node-version key in the workflow and
AGENTS.md references to make the corresponding change.
- Around line 18-19: The workflow step using the pnpm action ("Setup pnpm")
currently references the floating tag pnpm/action-setup@v4; replace that with a
pinned commit SHA (pnpm/action-setup@<full-commit-sha>) to prevent tag-mutation
supply-chain risks, updating the action reference in the same step and
optionally adding a short comment mentioning why the SHA is pinned so future
maintainers know it's intentional.
In @.github/workflows/claude-review.yml:
- Line 25: Replace the floating action reference
"anthropics/claude-code-action@beta" with a pinned version to ensure
reproducible workflow execution; update the uses entry to either the stable tag
"anthropics/[email protected]" or the specific commit SHA
"anthropics/claude-code-action@23ed4cb53d6eacddbc22ec16652c98bcc54e0476" in the
workflow file so the action is immutably pinned.
In `@AGENTS.md`:
- Line 91: Update the documentation text that currently says "pnpm deploy" to
the correct npm script invocation "pnpm run deploy" so the deploy script (node
src/deploy-commands.js) is executed; specifically replace the literal string
"pnpm deploy" with "pnpm run deploy" in the AGENTS.md line that instructs to run
the deploy script to register with Discord.
In `@CONTRIBUTING.md`:
- Around line 27-36: The fenced code block that lists commit message examples
(the block starting with the line "feat: add music playback command") lacks a
language tag and triggers markdownlint MD040; update the opening fence from ```
to ```text so the block becomes a plain-text fenced code block (i.e., add the
"text" language specifier to the existing commit examples block).
In `@coverage/clover.xml`:
- Around line 1-1121: The PR includes a generated coverage artifact
(coverage/clover.xml) that must not be committed; add the coverage/ directory to
.gitignore (so future coverage artifacts are ignored) and remove the currently
tracked file(s) from the repo index (e.g., git rm --cached coverage/clover.xml
and/or git rm -r --cached coverage/) so they are no longer tracked, then commit
the updated .gitignore and the removal; ensure the change targets the coverage/
directory and the specific tracked file coverage/clover.xml mentioned in the
diff.
In `@coverage/prettify.css`:
- Line 1: This commit includes generated coverage artifacts (e.g.,
coverage/prettify.css and coverage/block-navigation.js); add coverage/ to
.gitignore and remove the committed coverage files from the repo so they aren’t
tracked (use git rm --cached on the files/dir or revert the commit that added
them), then commit the updated .gitignore; ensure CI still produces/upload
coverage as an artifact rather than committing the coverage/ directory.
In `@coverage/sorter.js`:
- Around line 1-210: The committed file coverage/sorter.js (contains the
addSorting IIFE and functions like loadColumns/loadData) is a generated Istanbul
artifact causing Biome lint failures due to var usage; remove the generated
coverage/ output from version control (git rm --cached -r coverage, add
coverage/ to .gitignore) or, if you cannot remove it immediately, update
biome.json to ignore the directory (add an exclude entry for coverage/**) so the
auto-generated sorter.js (and other coverage files) are not linted by Biome.
In `@coverage/src/logger.js.html`:
- Around line 1-808: The repo is tracking the generated coverage/ artifacts; add
"coverage/" to .gitignore and stop tracking existing files by removing them from
the index. Update the .gitignore to include the coverage/ directory (and any
related coverage output you want ignored), then run git rm --cached -r coverage
to untrack the current files and commit the change; verify the coverage/ folder
is no longer shown in git status. Ensure you do not delete the local coverage
files—only stop tracking them—and include a brief commit message referencing
.gitignore and untracked coverage.
In `@coverage/src/utils/health.js.html`:
- Around line 1-562: Add "coverage/" to .gitignore and stop tracking the
generated artifacts under the coverage directory (e.g.
coverage/src/utils/health.js.html and other HTML/CSS/JS/JSON/XML/images produced
by Istanbul/Vitest). Update .gitignore to include the coverage/ entry, remove
the currently tracked coverage files from Git's index so they are no longer
committed, and then commit the .gitignore change and the removal of those
tracked coverage files.
In `@coverage/src/utils/index.html`:
- Around line 1-191: The repo is tracking the auto-generated coverage/ output
(examples: coverage/index.html, coverage/base.css, coverage/prettify.js,
coverage/sorter.js and many HTML/JSON/XML files); add an ignore rule for the
coverage/ directory to .gitignore (e.g., coverage/ or /coverage/) and remove all
tracked coverage files from git with git rm --cached for those files (or git rm
-r --cached coverage) then commit the .gitignore and removal, ensuring future
coverage artifacts (HTML, CSS, JS, JSON, XML) are not tracked.
In `@package.json`:
- Around line 12-16: CI is failing due to 348 Biome lint errors reported by the
"lint" script; run the "lint:fix" script (pnpm run lint:fix or npm run lint:fix)
to auto-fix issues, review any remaining errors/warnings reported by the "lint"
(biome check) command, fix residual problems manually, then commit the updated
formatting/lint changes so the "lint" and "test" scripts pass in CI; ensure
"format" (biome format) is used if needed and re-run CI locally before pushing.
In `@README.md`:
- Around line 21-34: Add a language identifier to the fenced code block
containing the ASCII architecture diagram (the triple-backtick block that starts
with "Discord User" and the box/arrow diagram) to satisfy markdownlint MD040;
update the opening fence from ``` to ```text (or ```plaintext) so the block is
treated as plain text while leaving the diagram content unchanged.
In `@src/commands/config.js`:
- Around line 299-305: The embed fields for the success message use raw
user-supplied path and truncatedValue when constructing the EmbedBuilder in the
Config command; sanitize these before embedding by passing both path and
truncatedValue through the existing escapeInlineCode helper (the same sanitizer
used in handleView/section validation) and use the escaped results when calling
.addFields on the EmbedBuilder so backticks or other markdown in
path/truncatedValue cannot break Discord formatting.
In `@src/deploy-commands.js`:
- Around line 44-49: When iterating commandFiles in deploy-commands.js,
currently files missing the expected exports are silently skipped; update the
loop that imports each module (the const command = await
import(join(commandsPath, file))) to call the same warning used in src/index.js
when command.data or command.execute are absent—emit a warning like "Command
missing data or execute export" and include the file name (file) in the log
payload before continuing, so skipped command files are visible during
deployment.
In `@src/logger.js`:
- Around line 142-150: The consoleFormat currently calls level.toUpperCase(),
which corrupts ANSI color codes added by colorize(); change the formatting to
use originalLevel.toUpperCase() for the text label (keeping
EMOJI_MAP[originalLevel] for the prefix) so you don't mutate the colorized
string, or alternatively move any toUpperCase() transformation to occur before
colorize() in the format pipeline; update consoleFormat to reference
originalLevel rather than level when creating the uppercased label.
In `@src/modules/events.js`:
- Around line 63-68: The spam warning currently logs PII and message content via
warn('Spam detected', { user: message.author.tag, content:
message.content.slice(0, 50) }); change this to avoid storing PII by removing
raw tag/content and instead log a non-PII identifier (e.g., a hashed or
truncated message.author.id) and non-sensitive metadata (e.g., content length or
a redacted indicator). Update the warn call in the spam block (where
config.moderation?.enabled and isSpam are checked) to use the anonymized
identifier and metadata; leave sendSpamAlert(message, client, config) behavior
intact if it needs full context. Ensure any helper used for hashing/obfuscation
is deterministic and referenced where you modify the warn invocation.
- Around line 122-124: The catch handler for the Promise returned by accumulate
currently logs err.message directly which can throw if a non-Error is rejected;
update the handler so it uses optional chaining (err?.message) when calling
logError to match the unhandled rejection handler and avoid throwing on
non-Error rejections—modify the callback passed to accumulate(...).catch to call
logError('ChimeIn accumulate error', { error: err?.message }) instead.
- Around line 20-21: The listener uses the wrong event name 'clientReady' so it
never fires; update the event registration in the client.once call to use the
correct Discord.js v14 event string ('ready') or use Events.ClientReady, e.g.,
replace the literal 'clientReady' with 'ready' (or Events.ClientReady) so the
handler that calls info(`${client.user.tag} is online`, { servers:
client.guilds.cache.size }) runs on startup.
In `@src/modules/welcome.js`:
- Around line 98-100: The catch block currently only logs err.message via
logError('Welcome error', { error: err.message }); — update that catch in
src/modules/welcome.js to include the error stack (or the full error object) in
the log so you retain stack traces for debugging (use logError('Welcome error',
{ error: err.message, stack: err.stack }) or pass err as the error payload);
keep the existing message context and ensure this change applies to the catch
handling around the welcome flow where logError is called.
In `@tests/commands.test.js`:
- Around line 16-33: The tests share a mutable variable mod between two it
blocks causing ordering/hidden-dependency issues; move the module import into a
per-spec setup (use beforeAll or beforeEach inside the describe for each
command) so mod is imported once before the it blocks run (replace the pattern
that assigns mod in the first it and uses mod = mod || import(join(commandsDir,
file)) with a setup that does mod = await import(join(commandsDir, file)) in
beforeAll/beforeEach), ensuring both tests reference the reliably-initialized
mod and removing the conditional re-import.
- Around line 20-21: The dynamic import call in tests (import(join(commandsDir,
file)) in tests/commands.test.js) triggers module-level side effects from
logger.js (which reads config.json, checks LOG_LEVEL and DATABASE_SSL, and may
create logs/), so before performing the dynamic import, ensure the test sets up
a safe environment: create or stub a minimal config.json and set environment
variables LOG_LEVEL and DATABASE_SSL to safe values, or alternatively mock/stub
logger.js (or the transitive imports like config.js/status.js) so the real
logger code is not executed; ensure this setup/teardown runs before/after the
import in the test to avoid filesystem or env leakage in CI.
In `@tests/commands/ping.test.js`:
- Around line 163-188: The test currently only checks that editReply was called;
update it to also assert the displayed latency is sensible (non-negative) by
inspecting the argument passed to editReply from execute(interaction): e.g.,
verify mockEditReply was called with an object/string containing a latency like
"0ms" or "\d+ms" and not a negative value (assert the string does not contain
'-' using expect.stringMatching and/or expect.not.stringMatching); reference the
execute function and the mockEditReply interaction to locate where to add this
assertion.
- Around line 20-216: Extract a reusable mock factory (e.g.,
createMockInteraction) to build the interaction object used by tests instead of
repeating mockReply, mockEditReply and the interaction structure in each spec;
the helper should accept parameters like messageTimestamp, interactionTimestamp,
and wsPing and return { reply: mockReply, editReply: mockEditReply,
createdTimestamp, client: { ws: { ping } } } so each test can call
createMockInteraction(...) and then await execute(interaction) and assert on
interaction.editReply or interaction.reply as before.
In `@tests/config.test.js`:
- Around line 9-59: Add a beforeAll hook inside the describe('config.json', ...)
block that reads the file with readFileSync(configPath, 'utf-8') and parses it
once into the outer-scoped config variable, then remove the repeated
read/JSON.parse lines from each it(...) test so they use the shared config;
ensure you do not re-declare or shadow config inside any test and keep
assertions (e.g., checks against config.ai, config.welcome, config.moderation,
config.permissions, config.logging) unchanged.
In `@tests/db.test.js`:
- Around line 46-128: Save the original process.env.DATABASE_URL before each
test and restore it after each test to avoid leaking env state between tests;
modify the tests in this file (the initDb describe block) to capture the current
value (e.g., in beforeEach store originalUrl = process.env.DATABASE_URL) and
restore it in afterEach, ensuring tests that set process.env.DATABASE_URL reset
it back so the "should throw error if DATABASE_URL is not set" test remains
isolated even if ordering changes; keep existing vi.resetModules() behavior but
add the env save/restore around tests that call db.initDb()/SSL variations.
In `@tests/logger.test.js`:
- Around line 50-59: The test only verifies logger.info doesn't throw but
doesn't assert actual redaction; update the test that calls logger.info to
capture the emitted log message (e.g., attach a temporary/test Winston transport
or spy the logger's transport) and assert that sensitive keys like
DISCORD_TOKEN, password, and apiKey are replaced with "[REDACTED]". Locate
usages of logger.info in tests/logger.test.js and reference the
redactSensitiveData logic invoked by the logger; ensure the test inspects the
transport's last log entry (message or meta) and checks redaction rather than
only asserting no throw.
In `@tests/modules/ai.test.js`:
- Around line 306-321: The test for including the authorization header is
incomplete: it never sets the module-level OPENCLAW_TOKEN (evaluated at import)
nor asserts the Authorization header on the fetch call. Fix by resetting module
cache with vi.resetModules(), set process.env.OPENCLAW_API_KEY (or
OPENCLAW_TOKEN if that’s the module variable) before dynamically importing the
module that exports generateResponse, then call generateResponse and assert that
mockFetch.mock.calls[0][1].headers['Authorization'] equals the expected bearer
token while keeping the Content-Type assertion.
- Around line 109-338: Tests directly assign global.fetch = mockFetch which
isn't reverted by vi.restoreAllMocks(); replace those assignments with
vi.stubGlobal('fetch', mockFetch) in each spec that sets a mock so the stubs are
tracked and automatically cleaned up by vi.restoreAllMocks() (keep references to
the same mockFetch variable used in each test), and ensure afterEach still calls
vi.restoreAllMocks() so generateResponse-related tests are isolated.
In `@tests/modules/welcome.test.js`:
- Around line 60-232: Tests for recordCommunityActivity only assert "does not
throw" and fail to verify state changes; add a way to observe guildActivity or
test via downstream behavior. Modify the module that defines
recordCommunityActivity to either export a read-only accessor (e.g.,
getGuildActivity) that returns the current guildActivity snapshot for
assertions, or make sendWelcomeMessage/getCommunitySnapshot testable and assert
their outputs after invoking recordCommunityActivity; update tests to call
recordCommunityActivity(...) then assert on getGuildActivity(guildId) or the
appropriate downstream function to verify messages are counted, bots/excluded
channels are ignored, separate channels tracked, and old entries pruned after
advancing timers.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
coverage/favicon.pngis excluded by!**/*.pngcoverage/sort-arrow-sprite.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (74)
.editorconfig.env.example.github/workflows/ci.yml.github/workflows/claude-review.ymlAGENTS.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdbiome.jsonconfig.jsoncoverage/base.csscoverage/block-navigation.jscoverage/clover.xmlcoverage/coverage-final.jsoncoverage/index.htmlcoverage/prettify.csscoverage/prettify.jscoverage/sorter.jscoverage/src/commands/config.js.htmlcoverage/src/commands/index.htmlcoverage/src/commands/ping.js.htmlcoverage/src/commands/status.js.htmlcoverage/src/db.js.htmlcoverage/src/index.htmlcoverage/src/index.js.htmlcoverage/src/logger.js.htmlcoverage/src/modules/ai.js.htmlcoverage/src/modules/chimeIn.js.htmlcoverage/src/modules/config.js.htmlcoverage/src/modules/events.js.htmlcoverage/src/modules/index.htmlcoverage/src/modules/spam.js.htmlcoverage/src/modules/welcome.js.htmlcoverage/src/utils/errors.js.htmlcoverage/src/utils/health.js.htmlcoverage/src/utils/index.htmlcoverage/src/utils/permissions.js.htmlcoverage/src/utils/registerCommands.js.htmlcoverage/src/utils/retry.js.htmlcoverage/src/utils/splitMessage.js.htmlpackage.jsonsrc/commands/config.jssrc/commands/ping.jssrc/commands/status.jssrc/deploy-commands.jssrc/index.jssrc/logger.jssrc/modules/ai.jssrc/modules/chimeIn.jssrc/modules/config.jssrc/modules/events.jssrc/modules/spam.jssrc/modules/welcome.jssrc/utils/errors.jssrc/utils/registerCommands.jssrc/utils/retry.jstest-log-levels.jstests/commands.test.jstests/commands/ping.test.jstests/config.test.jstests/db.test.jstests/logger.test.jstests/modules/ai.test.jstests/modules/spam.test.jstests/modules/welcome.test.jstests/utils/errors.test.jstests/utils/health.test.jstests/utils/permissions.test.jstests/utils/retry.test.jstests/utils/splitMessage.test.jsverify-contextual-logging.jsverify-file-output.jsverify-sensitive-data-redaction.jsvitest.config.js
💤 Files with no reviewable changes (1)
- test-log-levels.js
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json,css,md} : After changing or editing any files, run the complete validation workflow: `pnpm format && pnpm typecheck && pnpm lint && pnpm build` before committing
Applied to files:
.github/workflows/ci.ymlbiome.json
📚 Learning: 2026-02-04T02:20:09.131Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-04T02:20:09.131Z
Learning: Applies to {package.json,.npmrc,pnpm-lock.yaml} : Use pnpm package manager (pinned to v10.23.0) with `.npmrc` configured for strict peer dependencies and disabled shamefully-hoist
Applied to files:
package.json
📚 Learning: 2025-10-10T15:05:26.145Z
Learnt from: CR
Repo: BillChirico/LUA-Obfuscator PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-10T15:05:26.145Z
Learning: Applies to package.json : Only add new packages when absolutely necessary or explicitly requested
Applied to files:
package.json
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to tests/**/*.test.{ts,tsx} : Use Node.js test runner with `tsx` for unit tests located in `tests/` directory; run with `pnpm test`
Applied to files:
tests/commands.test.js
📚 Learning: 2026-02-04T02:20:09.131Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-04T02:20:09.131Z
Learning: Applies to src/**/*.{ts,tsx} : Use `reportError(context, error)` from `src/lib/logger.ts` to report errors to Sentry with context metadata, falling back to console.error if Sentry is disabled
Applied to files:
coverage/src/utils/errors.js.html
🧬 Code graph analysis (14)
tests/modules/welcome.test.js (1)
src/modules/welcome.js (2)
renderWelcomeMessage(25-31)recordCommunityActivity(39-71)
src/deploy-commands.js (4)
src/index.js (8)
__filename(29-29)__dirname(30-30)token(288-288)guildId(153-153)commands(152-152)command(133-133)command(165-165)command(194-194)src/logger.js (1)
__dirname(17-17)src/utils/registerCommands.js (1)
registerCommands(19-57)src/modules/config.js (1)
err(30-30)
tests/config.test.js (1)
tests/commands.test.js (1)
__dirname(6-6)
src/utils/registerCommands.js (1)
src/logger.js (1)
info(216-218)
tests/db.test.js (1)
src/db.js (1)
pg(9-9)
tests/modules/spam.test.js (1)
src/modules/spam.js (2)
isSpam(26-28)sendSpamAlert(36-61)
tests/commands/ping.test.js (1)
src/index.js (1)
interaction(178-178)
tests/modules/ai.test.js (1)
src/modules/ai.js (5)
setConversationHistory(24-26)getConversationHistory(16-18)getHistory(42-47)addToHistory(55-63)generateResponse(74-145)
src/index.js (2)
src/utils/registerCommands.js (1)
registerCommands(19-57)src/logger.js (1)
error(230-232)
src/modules/welcome.js (1)
src/logger.js (1)
info(216-218)
src/modules/events.js (1)
src/logger.js (2)
info(216-218)warn(223-225)
src/modules/config.js (1)
src/db.js (1)
client(82-82)
tests/logger.test.js (1)
src/logger.js (2)
logger(200-204)error(230-232)
src/commands/config.js (2)
src/index.js (1)
config(43-43)src/modules/config.js (3)
section(150-150)sectionData(270-270)err(30-30)
🪛 GitHub Actions: CI
coverage/block-navigation.js
[error] 10-10: pnpm lint: lint/style/useTemplate. Use template literals instead of string concatenation. See log: var notSelector = ':not(' + missingCoverageClasses.join('):not(') + ') > ';
[error] 2-2: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found in: var jumpToCode = (function init() {
[error] 4-4: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var missingCoverageClasses = ['.cbranch-no', '.cstat-no', '.fstat-no'];
[error] 7-7: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var fileListingElements = ['td.pct.low'];
[error] 13-13: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var selector =
[error] 20-20: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var missingCoverageElements = document.querySelectorAll(selector);
[error] 22-22: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var currentIndex;
[error] 42-42: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var nextIndex = 0;
[error] 53-53: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var nextIndex = 0;
coverage/sorter.js
[error] 2-2: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var addSorting = (function() {
[error] 60-60: pnpm lint: lint/suspicious/noVar. Use let or const instead of var. Found: var template = document.getElementById('filterTemplate');
package.json
[warning] 1-1: pnpm lint: Lint reported issues exceed display limit (Diagnostics not shown). 348 errors, 33 warnings.
🪛 markdownlint-cli2 (0.20.0)
CONTRIBUTING.md
[warning] 27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
[warning] 21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
AGENTS.md
[warning] 90-90: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/2/3
(MD029, ol-prefix)
[warning] 91-91: Ordered list item prefix
Expected: 2; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 92-92: Ordered list item prefix
Expected: 3; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 105-105: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 106-106: Ordered list item prefix
Expected: 2; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
🪛 Stylelint (17.2.0)
coverage/base.css
[error] 54-54: Expected "0.5" to be "50%" (alpha-value-notation)
(alpha-value-notation)
[error] 47-50: Expected empty line before at-rule (at-rule-empty-line-before)
(at-rule-empty-line-before)
[error] 132-132: Unexpected empty block (block-no-empty)
(block-no-empty)
[error] 139-139: Unexpected empty block (block-no-empty)
(block-no-empty)
[error] 54-54: Expected "rgba" to be "rgb" (color-function-alias-notation)
(color-function-alias-notation)
[error] 54-54: Expected modern color-function notation (color-function-notation)
(color-function-notation)
[error] 169-169: Expected modern color-function notation (color-function-notation)
(color-function-notation)
[error] 171-171: Expected modern color-function notation (color-function-notation)
(color-function-notation)
[error] 173-173: Expected modern color-function notation (color-function-notation)
(color-function-notation)
[error] 174-174: Expected modern color-function notation (color-function-notation)
(color-function-notation)
[error] 155-155: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 157-157: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 164-164: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 166-166: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 168-168: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 170-170: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 172-172: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 175-175: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 178-178: Expected empty line before comment (comment-empty-line-before)
(comment-empty-line-before)
[error] 16-16: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 26-26: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 48-48: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 156-156: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 181-181: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 182-182: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 183-183: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 214-214: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 6-6: Expected quotes around "Helvetica Neue" (font-family-name-quotes)
(font-family-name-quotes)
[error] 6-6: Unexpected missing generic font family (font-family-no-missing-generic-family-keyword)
(font-family-no-missing-generic-family-keyword)
[error] 146-146: Expected quotes around "url" function argument (function-url-quotes)
(function-url-quotes)
[error] 47-47: Expected "context" media feature range notation (media-feature-range-notation)
(media-feature-range-notation)
[error] 12-12: Unexpected vendor-prefixed property "-webkit-box-sizing" (property-no-vendor-prefix)
(property-no-vendor-prefix)
[error] 13-13: Unexpected vendor-prefixed property "-moz-box-sizing" (property-no-vendor-prefix)
(property-no-vendor-prefix)
[error] 22-22: Unexpected vendor-prefixed property "-moz-tab-size" (property-no-vendor-prefix)
(property-no-vendor-prefix)
[error] 23-23: Unexpected vendor-prefixed property "-o-tab-size" (property-no-vendor-prefix)
(property-no-vendor-prefix)
[error] 5-9: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 11-15: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 18-25: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 39-45: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 69-73: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 80-83: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 84-88: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 95-103: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 113-115: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 116-119: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 126-130: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 133-136: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 148-150: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 151-153: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 160-163: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 199-201: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 202-204: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 205-207: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 208-212: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 222-224: Expected empty line before rule (rule-empty-line-before)
(rule-empty-line-before)
[error] 11-11: Expected double colon pseudo-element notation (selector-pseudo-element-colon-notation)
(selector-pseudo-element-colon-notation)
[error] 11-11: Expected double colon pseudo-element notation (selector-pseudo-element-colon-notation)
(selector-pseudo-element-colon-notation)
[error] 39-39: Expected double colon pseudo-element notation (selector-pseudo-element-colon-notation)
(selector-pseudo-element-colon-notation)
[error] 29-29: Expected "10px 0 0 0" to be "10px 0 0" (shorthand-property-no-redundant-values)
(shorthand-property-no-redundant-values)
[error] 71-71: Expected "10px 0 0 0" to be "10px 0 0" (shorthand-property-no-redundant-values)
(shorthand-property-no-redundant-values)
coverage/prettify.css
[error] 1-1: Expected empty line before at-rule (at-rule-empty-line-before)
(at-rule-empty-line-before)
[error] 1-1: Expected empty line before at-rule (at-rule-empty-line-before)
(at-rule-empty-line-before)
[error] 1-1: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 1-1: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 1-1: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 1-1: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 1-1: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 1-1: Expected no more than 1 declaration (declaration-block-single-line-max-declarations)
(declaration-block-single-line-max-declarations)
[error] 1-1: Unexpected deprecated media type "projection" (media-type-no-deprecated)
(media-type-no-deprecated)
[error] 1-1: Expected class selector ".L0" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L1" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L2" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L3" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L5" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L6" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L7" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L8" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L1" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L3" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L5" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L7" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
[error] 1-1: Expected class selector ".L9" to be kebab-case (selector-class-pattern)
(selector-class-pattern)
🔇 Additional comments (40)
src/commands/ping.js (1)
7-17: LGTM — clean formatting improvements.The trailing comma and template literal consolidation are minor style fixes with no behavioral change.
src/modules/chimeIn.js (1)
12-14: LGTM — import and formatting cleanup only.No behavioral changes; the reformatted lines are consistent with the project's Biome style.
.editorconfig (1)
1-17: LGTM — sensible defaults.Settings align with Biome's configuration (2-space indent, LF endings). The Markdown and Makefile overrides are appropriate.
src/utils/registerCommands.js (1)
19-56: LGTM — clean command registration utility.Good input validation, proper logging, and the guild/global branching is straightforward. Rethrowing after logging preserves caller error handling.
AGENTS.md (1)
27-27: Fenced code block on line 27 should specify a language.The static analysis hint from markdownlint (MD040) flags that fenced code blocks should have a language specified. Oh wait, this is for CONTRIBUTING.md not AGENTS.md. Let me skip this.
CLAUDE.md (1)
1-3: LGTM — clean pointer to AGENTS.md.CONTRIBUTING.md (1)
1-77: LGTM — well-structured contribution guide.Clear workflow, consistent with the tooling and CI setup introduced in this PR.
src/commands/status.js (1)
61-152: LGTM — solid error handling and admin gating.The execute function correctly:
- Gates detailed diagnostics behind
PermissionFlagsBits.Administrator- Uses ephemeral replies for sensitive/admin output
- Handles both
replied/deferredstates in the catch block- Silently catches follow-up failures to prevent unhandled rejections
package.json (1)
33-37: Dependencies are at current stable versions.All three package versions are confirmed current as of February 2026:
@biomejs/[email protected]is the latest stable release (published Feb 3, 2026)[email protected]is the latest stable release (published Jan 22, 2026)@vitest/[email protected]is the latest stable releaseNo updates are needed.
.env.example (1)
1-34: Well-structured environment template with clear documentation.The required/optional distinction is clear, backward-compatible aliases are well-documented as comments, and the grouping is logical.
src/logger.js (2)
51-59: Good addition ofOPENCLAW_API_KEYto the redaction list.This aligns the redaction set with the new standardized environment variable names introduced in this PR.
25-34: Synchronous file I/O at module load blocks the event loop.
existsSyncandreadFileSyncat lines 26-27 run synchronously during module initialization. For a startup-time config load this is generally acceptable, but worth noting that this blocks the event loop until the file is read and parsed. Since this only runs once at startup, the impact is minimal.biome.json (2)
2-2: [No action required] The Biome schema version2.3.14already matches the installed@biomejs/biomeversion (^2.3.14) in devDependencies.Likely an incorrect or invalid review comment.
41-43: Use negated patterns infiles.includesto exclude generated directories.Biome v2.x doesn't support a
files.ignoreskey. Instead, exclude directories using negated patterns (prefix!) withinfiles.includes, or enable VCS integration to respect.gitignore.♻️ Proposed addition
"files": { - "includes": ["**/*.js", "**/*.json", "**/*.md"] + "includes": ["**/*.js", "**/*.json", "**/*.md", "!**/coverage/**", "!**/dist/**"] }Alternatively, enable VCS integration to automatically respect
.gitignoreif those directories are already listed:{ "vcs": { "enabled": true, "clientKind": "git", "useIgnoreFile": true } }Likely an incorrect or invalid review comment.
coverage/src/utils/splitMessage.js.html (1)
1-268: Addcoverage/to.gitignoreto prevent committing auto-generated reports.The
coverage/directory contains auto-generated Istanbul HTML reports. These files should not be tracked in version control as they bloat the repository, create unnecessary merge conflicts, and become stale immediately. Addcoverage/to.gitignore:+# Coverage reports +coverage/src/modules/config.js (3)
6-10: Good modernization of imports.The
node:prefix for built-in modules and the clean logger import aliasing look good.
96-105: Good improvement: resilient rollback handling.Wrapping
ROLLBACKin a try-catch prevents an unhandled rejection if the connection is already broken (e.g., network drop during a failed transaction). This pattern is applied consistently across all three transaction blocks in the file.
340-346: Solid prototype pollution protection.The
DANGEROUS_KEYScheck invalidatePathSegmentsplus the redundant checks insidesetNestedValueprovide defense-in-depth against__proto__/constructor/prototypeinjection through dot-notation config paths.config.json (1)
26-26: Formatting-only change — LGTM.src/modules/welcome.js (1)
6-7: Good adoption of centralized logger.Replacing console calls with the structured logger aligns with the project-wide logging standardization.
src/modules/spam.js (1)
26-61: Formatting-only changes — LGTM.The minor stylistic adjustments (arrow function parentheses, lowercase hex color, line breaks) don't alter behavior.
tests/modules/spam.test.js (1)
1-339: Comprehensive and well-structured test suite.Good coverage of both happy paths and edge cases (empty content, fetch failures, delete errors, missing config). The mock patterns are clean and consistent.
tests/logger.test.js (1)
80-83: Good edge-case coverage for null/undefined metadata.Passing
nulloverrides themeta = {}default parameter. This is a useful smoke test since Winston's splat format can behave unexpectedly with null metadata.src/modules/ai.js (1)
6-6: LGTM on the env var aliasing, import refactoring, and function signature.The backward-compatible env var resolution (
OPENCLAW_API_URL→OPENCLAW_URL→ default) is clean, and switching fromconsole.errorto the structuredlogErroralias is a good improvement.Also applies to: 28-35, 74-80
tests/modules/welcome.test.js (2)
9-57: Good coverage ofrenderWelcomeMessagewith meaningful assertions.Tests verify each placeholder, multiple occurrences, missing username fallback, no-placeholder pass-through, and empty templates. Well-structured.
235-261: LGTM — edge case coverage forrenderWelcomeMessage.Long server names, large member counts, special characters, and numeric usernames are all useful boundary cases.
tests/modules/ai.test.js (2)
21-106: Well-structured history management and trimming tests.Good coverage of
getConversationHistory,setConversationHistory,getHistory, andaddToHistoryincluding the MAX_HISTORY=20 boundary.
109-210: Good coverage ofgenerateResponsecore paths.API call verification, system prompt inclusion, history threading, post-success history update, and both HTTP and network error fallbacks are well tested.
tests/db.test.js (2)
183-242: Good SSL configuration test coverage.The tests properly verify all four SSL paths: railway.internal detection, explicit
false/off,no-verifymode, and the secure default. Each inspects the actual Pool constructor arguments.
145-181: LGTM —closeDband re-initialization tests are solid.Covers graceful close, error swallowing, safe no-op when uninitialized, and re-init after close. Good lifecycle coverage.
src/commands/config.js (2)
76-114: Well-structured recursive path collection with good edge-case handling.The function correctly handles empty arrays, empty objects, and nested structures, and emits only leaf paths for autocomplete. Clean implementation.
18-64: Command builder definition looks correct for discord.js v14.Subcommands, options, autocomplete flags, and required flags are all properly configured.
src/utils/errors.js (1)
118-156: Formatting-only changes to message strings look good.src/utils/retry.js (2)
8-9: Import reorder and style changes look good.No functional changes — just import order and minor formatting adjustments.
27-33:2 ** attemptis a clean replacement forMath.pow(2, attempt).Equivalent semantics, more idiomatic modern JS.
coverage/src/modules/config.js.html (1)
1-1411:andsrc/deploy-commands.js (1)
1-62: Overall: clean deployment script, LGTM with the minor suggestion above.Environment validation, backward-compatible env aliases, and error handling are well-structured.
src/index.js (3)
14-26: Good practice: explicitnode:protocol for built-in imports.Using
node:fs,node:path, andnode:urlis the recommended approach in modern Node.js — it avoids ambiguity with user-land packages and makes the import intent clear.
86-103:saveStatesilently swallows write errors during shutdown.The
catchblock logs the error but doesn't propagate it. DuringgracefulShutdown, the caller has no way to know state wasn't saved. This is acceptable for best-effort persistence, but worth noting — if state loss is critical, consider re-throwing or returning a success boolean.
303-330: Startup sequence looks solid.Clean sequential initialization: DB → config → state → event handlers → commands → login. The
.catchat Line 327 withprocess.exit(1)ensures the bot doesn't hang if initialization fails.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Pin actions/checkout in CI and Claude review workflows - Pin actions/setup-node in CI workflow - Fix claude-code-action pin to dereferenced v1.0.48 commit SHA
- Clarify undici override note for Node 22 context - Update README Node.js badge and prerequisite to 22+
- Use generic error message in production for view/set/reset paths - Keep detailed err.message in non-production - Escape section name in reset embed description
|
✅ Addressed all 14 unresolved Round 2 review threads on What was fixed
Validation
All listed review threads were resolved via GraphQL. |
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
Or push these changes by commenting: |
| preserveOriginalLevel, | ||
| winston.format.colorize(), | ||
| winston.format.timestamp({ format: 'YYYY-MM-DD HH:mm:ss' }), | ||
| consoleFormat |
There was a problem hiding this comment.
Colorize format now produces no visible effect
Low Severity
winston.format.colorize() is applied in the console transport format chain, but the colorized level variable is destructured and discarded in consoleFormat. The output string uses originalLevel.toUpperCase() instead, so the colorize format runs without any visible effect on the output. This is dead code that adds unnecessary processing.
- Test refreshDiscordToken: success, failure, token rotation, no refresh token (issue #15) - Test fetchWithRateLimit: 429 response, retry-after parsing, max retries exhaustion (issue #16) - Test proxy: actual redirect behavior for unauthenticated requests, passthrough for valid tokens, callbackUrl preservation (issue #17) - Fix mock types: complete Account/Token objects with all required fields (issue #18) - Update guilds API test to use getToken() instead of session.accessToken - Update landing page tests for conditional invite button rendering - Update header/dashboard-shell/server-selector tests for refactored components - Add health check endpoint test - All 82 tests passing, lint clean, build succeeds
- Differentiate requireGuildAdmin (ADMINISTRATOR only) from requireGuildModerator (ADMINISTRATOR | MANAGE_GUILD), aligning REST admin check with slash-command isAdmin (#1, #2, #12) - Add botOwners startup warning when using default upstream ID (#3) - Add SESSION_SECRET, DISCORD_CLIENT_SECRET, DISCORD_REDIRECT_URI to README deployment table (#4) - Pass actual permission level to getPermissionError so modlog denial says 'moderator' not 'administrator' (#5) - Guard _seedOAuthState with NODE_ENV production check (#6) - Add test: valid JWT with no server-side session (#7) - Add DiscordApiError class with HTTP status (#8) - Add moderatorRoleId support to isModerator (#9) - Remove no-op delete override from SessionStore (#10) - Cap oauthStates at 10k entries (#11) - Fix hasOAuthGuildPermission docstring for bitwise OR semantics (#12) - Handle dashboard URL fragment collision (#13) - Cap guildCache at 10k entries (#14) - Add SESSION_TTL_MS co-location comment with JWT expiry (#15) - Cache SESSION_SECRET via lazy getter in verifyJwt (#16) - Remove PII (username) from OAuth auth log (#17)
- Differentiate requireGuildAdmin (ADMINISTRATOR only) from requireGuildModerator (ADMINISTRATOR | MANAGE_GUILD), aligning REST admin check with slash-command isAdmin (#1, #2, #12) - Add botOwners startup warning when using default upstream ID (#3) - Add SESSION_SECRET, DISCORD_CLIENT_SECRET, DISCORD_REDIRECT_URI to README deployment table (#4) - Pass actual permission level to getPermissionError so modlog denial says 'moderator' not 'administrator' (#5) - Guard _seedOAuthState with NODE_ENV production check (#6) - Add test: valid JWT with no server-side session (#7) - Add DiscordApiError class with HTTP status (#8) - Add moderatorRoleId support to isModerator (#9) - Remove no-op delete override from SessionStore (#10) - Cap oauthStates at 10k entries (#11) - Fix hasOAuthGuildPermission docstring for bitwise OR semantics (#12) - Handle dashboard URL fragment collision (#13) - Cap guildCache at 10k entries (#14) - Add SESSION_TTL_MS co-location comment with JWT expiry (#15) - Cache SESSION_SECRET via lazy getter in verifyJwt (#16) - Remove PII (username) from OAuth auth log (#17)
… (#83) * feat(dashboard): add Discord entity pickers Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(dashboard): add diff view and polish - ConfigDiff.tsx: visual diff component showing before/after config changes with green additions and red deletions using the `diff` library - SystemPromptEditor.tsx: textarea with real-time character count, max length warning indicator, and accessible labeling - Toast notifications via sonner: success/error toasts on save, load failures, and reset actions positioned bottom-right - ResetDefaultsButton with confirmation dialog using Radix UI Dialog - ConfigEditor.tsx: full config editing page with AI, welcome message, and moderation sections; PATCH-based save with diff preview - Config API proxy route (GET/PATCH) following established analytics proxy pattern with guild admin authorization - Dialog UI component (shadcn/ui new-york style) - Added lint script to web/package.json Co-Authored-By: Claude Opus 4.6 <[email protected]> * polish(config-editor): improve UX, accessibility, and edge case handling - Replace native checkboxes with styled toggle switches using proper role="switch" and aria-checked attributes - Add unsaved changes guard (beforeunload warning + yellow banner) - Add Ctrl/Cmd+S keyboard shortcut for saving - Block save when system prompt exceeds character limit - Rename misleading "Reset to Defaults" to "Discard Changes" with accurate dialog copy (reverts to last saved state, not factory defaults) - Add diff summary counts (+N / -N) to the pending changes card - Improve accessibility throughout: aria-labels on loading spinner, aria-describedby linking textareas to their hints, aria-invalid on over-limit prompt, aria-live on character counter, aria-hidden on decorative icons, role="region" on diff view - Memoize hasChanges and hasValidationErrors to avoid redundant JSON.stringify on every render - Validate PATCH body shape in API route before proxying upstream - Fix stale "bills-bot" prefix in guild-selection localStorage keys (missed during volvox rename) Closes #31 Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(api): add config endpoints for Issue #31 Add REST API endpoints for managing bot configuration: - GET /api/v1/config - Retrieve current config (ai, welcome, spam, moderation) - PUT /api/v1/config - Update config with schema validation Features: - Type-safe schema validation for config sections - Flatten nested objects to dot-notation paths for persistence - requireGlobalAdmin middleware (API-secret or bot-owner OAuth) - Proper HTTP error codes (400 for validation, 401/403 for auth, 500 for errors) - Added PUT to CORS methods Tests: - 35 comprehensive tests covering auth, validation, types, edge cases - Tests for validateConfigSchema and flattenToLeafPaths exports Closes #31 * feat(api): add webhook notifications for config changes - Add notifyDashboardWebhook() fire-and-forget sender to PATCH /:id/config - POST /webhooks/config-update endpoint for dashboard to push config changes - Webhook uses DASHBOARD_WEBHOOK_URL env var with 5s timeout - Add comprehensive tests for webhook functionality * feat(dashboard): add config editor with Zustand state management Add a full config editor UI at /dashboard/config with: - Proxy API routes (GET + PATCH) for bot config at /api/guilds/:guildId/config - Zustand store for config state with fetch, update, and debounced saves - Accordion-based sections for ai, welcome, spam, moderation (read-only), triage - Recursive field renderer supporting booleans, numbers, strings, arrays, objects - shadcn/ui components: accordion, input, label, switch, textarea Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(dashboard): enhance Discord entity pickers with multi-select and Zustand - Add multi-select mode to ChannelSelector and RoleSelector (multiple?: boolean prop) - Create Zustand store for caching channels/roles per guild - Add dedicated bot API endpoints: GET /:id/channels and GET /:id/roles - Add Next.js proxy routes for channels and roles - Update AGENTS.md with new key files Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: remove unused import in webhooks.js * fix: resolve all linting errors across codebase - Migrate biome config from 2.3.14 to 2.4.0 - Fix unused imports (triage.js, modAction.test.js) - Fix import ordering (auth.js, lock.js, unlock.js, ai.js, triage-respond.js, modAction.js, modAction.test.js) - Fix formatting across 19 files - Replace O(n²) spread in reduce with push (cli-process.test.js) - Use Object.hasOwn() instead of Object.prototype.hasOwnProperty (config-guild.test.js) All 1310 tests pass. * feat: add full config editing support for moderation and triage - Add moderation and triage to SAFE_CONFIG_KEYS in guilds.js, webhooks.js, and config.js making them writable via PATCH/PUT endpoints - Expand READABLE_CONFIG_KEYS to include all sections: ai, welcome, spam, moderation, triage, logging, memory, permissions - Add CONFIG_SCHEMA definitions for moderation and triage sections with full type validation - Update WritableConfigSection type to include moderation and triage - Remove moderation from READ_ONLY_SECTIONS in config-section.tsx - Update config-store.ts writable keys check - Add editable moderation section in dashboard config-editor with toggles for enabled, autoDelete, DM notifications, and escalation - Add editable triage section with fields for models, budgets, intervals, streaming, debug footer, and moderation log channel - Update all test assertions to reflect new writable sections Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(security): add webhook URL validation, schema validation on all write paths, atomic writes - Add shared validateWebhookUrl() utility that blocks SSRF (localhost, private IPs, link-local, IPv6 loopback) and enforces https in production - Wire URL validation into config.js notifyWebhook and guilds.js notifyDashboardWebhook - Export validateSingleValue() from config.js and apply it to the PATCH endpoint in guilds.js and the POST /config-update webhook endpoint - Add path length (<=200 chars) and depth (<=10 segments) limits to guilds.js PATCH and webhooks.js POST endpoints - Refactor PUT handler in config.js to track per-field write results: returns 200 on full success, 207 on partial failure, 500 when all fail - Add comprehensive tests for all new validations and error responses Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: extract shared config allowlist, webhook utility, and proxy helpers; remove dead code Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(frontend): batch saves, fix race conditions, DRY constants, localStorage safety - Batch saveChanges into parallel PATCH requests grouped by section instead of sequential individual PATCHes (I5) - Add request sequence counter to Zustand config store to prevent stale PATCH responses from clobbering newer state (I6) - Centralize SYSTEM_PROMPT_MAX_LENGTH constant in types/config.ts and import in config-editor and system-prompt-editor (M2) - Wrap localStorage.getItem in try/catch for SSR safety (M3) - Fix channels.length / roles.length truthiness bug — use !== undefined instead of .length which is falsy for 0 (M5) - Replace JSON.stringify deep comparison with recursive deepEqual utility function (M8) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(security): mask sensitive config fields, block IPv4-mapped IPv6 SSRF, reject unknown config paths - Add SENSITIVE_FIELDS set and maskSensitiveFields utility to strip triage API keys (classifyApiKey, respondApiKey) from all GET config responses - Block SSRF via IPv4-mapped IPv6 addresses (::ffff:127.0.0.1, hex form ::ffff:7f00:1, and cloud metadata ::ffff:169.254.169.254) - Reject unknown config paths in validateSingleValue instead of silently accepting them without type checking - Add cache size limit (100 entries) to webhook URL validation cache - Guard flattenToLeafPaths against __proto__/constructor/prototype keys Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor(backend): extract shared validators and getBotOwnerIds, add webhook utility tests Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(frontend): remove dead code, fix save flow, harden inputs and type guards - C4: delete 10 unused files (stores, UI components, dashboard selectors) and remove zustand, @radix-ui/react-accordion, @radix-ui/react-label, @radix-ui/react-switch from package.json - m8: replace ~85-line local GuildConfig interface with DeepPartial<BotConfig> (add DeepPartial utility to types/config.ts) - m4: harden isGuildConfig type guard to verify at least one expected section key (ai, welcome, spam) instead of just typeof === "object" - M6: fix computePatches to include top-level paths (remove incorrect fullPath.includes(".") guard that silently dropped top-level field changes) - M7: fix partial save to merge only succeeded sections into savedConfig on partial failure, preserving draft edits for failed sections; only call fetchConfig() on full success - m5: add min constraints to triage number inputs (budgets min=0, timeouts min=1, buffer/context sizes min=1) - m9: add e.returnValue = "" to beforeunload handler for modern browser support Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * test: fix 10-segment test to use valid schema path after strict validation * fix: wire default-personality.md as system prompt fallback Replace generic 'You are a helpful Discord bot.' fallback with the existing default-personality.md template, which provides the intended Volvox Bot personality, role constraints, and anti-abuse guardrails. * fix: merge default-personality into responder system prompt, remove dead template Merge personality, role, constraints, and anti-abuse from the unused default-personality.md into triage-respond-system.md (the responder's actual system prompt). Revert the fallback wiring in triage-prompt.js since personality now lives in the system prompt file where it belongs. Delete default-personality.md — no longer needed. * 📝 Add docstrings to `feat/config-editor-combined` Docstrings generation was requested by @BillChirico. The following files were modified: * `src/api/routes/auth.js` * `src/api/routes/config.js` * `src/api/routes/guilds.js` * `src/api/utils/configAllowlist.js` * `src/api/utils/validateConfigPatch.js` * `src/api/utils/validateWebhookUrl.js` * `src/commands/lock.js` * `src/commands/slowmode.js` * `src/commands/unlock.js` * `src/modules/memory.js` * `src/modules/triage-prompt.js` * `src/modules/triage-respond.js` * `src/modules/triage.js` * `src/utils/debugFooter.js` * `src/utils/permissions.js` * `web/src/app/api/guilds/[guildId]/channels/route.ts` * `web/src/app/api/guilds/[guildId]/config/route.ts` * `web/src/app/api/guilds/[guildId]/roles/route.ts` * `web/src/app/dashboard/config/page.tsx` * `web/src/components/dashboard/config-diff.tsx` * `web/src/components/dashboard/config-editor.tsx` * `web/src/components/dashboard/reset-defaults-button.tsx` * `web/src/components/dashboard/system-prompt-editor.tsx` * `web/src/components/providers.tsx` * `web/src/lib/bot-api-proxy.ts` These files were kept as they were: * `src/api/server.js` * `src/api/utils/webhook.js` These files were ignored: * `tests/api/routes/config.test.js` * `tests/api/routes/guilds.test.js` * `tests/api/routes/webhooks.test.js` * `tests/api/utils/validateWebhookUrl.test.js` * `tests/api/utils/webhook.test.js` * `tests/commands/tempban.test.js` * `tests/config-listeners.test.js` * `tests/modules/cli-process.test.js` * `tests/modules/config-guild.test.js` * `tests/utils/modAction.test.js` These file types are not supported: * `.env.example` * `AGENTS.md` * `biome.json` * `src/prompts/triage-respond-system.md` * `web/package.json` * fix(prompts): replace ambiguous 'classified' with 'triaged' (Thread #11) * fix(prompts): define concrete 'flagging' mechanism for moderation (Thread #12) * fix(prompts): add PII/credentials constraint to prevent echoing secrets (Thread #13) * fix: remove bracketed-IPv6 dead code in webhook URL validator URL.hostname already strips brackets from IPv6 addresses (new URL('http://[::1]').hostname === '::1'), so the hostname.startsWith('[') branch was unreachable dead code. Remove the bracketed-IPv6 branch and the '[::1]' entry from BLOCKED_IPV6 since it could never match. Addresses CodeRabbit review thread #9. * fix: sanitize webhook URL in warning logs to prevent credential exposure Strip query string and fragment from webhook URLs before including them in warning log messages. If a webhook URL contains tokens or API keys as query parameters, they would previously appear in logs. Now logs only origin + pathname (e.g. 'https://example.com/hook') instead of the full URL with sensitive query params. Addresses CodeRabbit review thread #10. * test(config): rename misleading 'Infinity' test name (Thread #14) * fix: normalize validateSingleValue to always return string[] Unknown config paths previously returned [{path, message}] objects while validateValue returned string[]. Callers now receive a consistent string[] in both cases, eliminating the need to handle two different shapes. * test(config): move PUT partial write handling inside config routes suite (Thread #15) * refactor: extract getGuildChannels helper to remove duplication GET /:id and GET /:id/channels shared identical channel-fetching loops with a duplicated MAX_CHANNELS constant. Extracted into a single getGuildChannels(guild) helper used by both handlers. * feat: include section (topLevelKey) in PATCH config webhook payload Downstream consumers of the DASHBOARD_WEBHOOK_URL can now use the 'section' field to selectively reload only the affected config section rather than refreshing the entire config. * fix: return promise in webhook config-update handler (async/await) The previous .then()/.catch() chain was not returned, so Express 5 could not auto-forward rejected promises to the error handler. Converted to async/await so errors propagate correctly. * feat: make JSON body size limit configurable via API_BODY_LIMIT env var Defaults to '100kb' when the env var is not set, preserving existing behaviour. Operators can now tune the limit without code changes. * test(validateWebhookUrl): strengthen cache eviction test to verify re-evaluation (Thread #16) * test(webhook): move vi.useRealTimers() to afterEach to prevent timer leak (Thread #17) * refactor: move CONFIG_SCHEMA/validateValue/validateSingleValue to utils/configValidation.js validateConfigPatch.js (utils) was importing validateSingleValue from routes/config.js — an inverted dependency. Created src/api/utils/configValidation.js as the canonical home for CONFIG_SCHEMA, validateValue, and validateSingleValue. - config.js now imports from ../utils/configValidation.js and re-exports validateSingleValue for backward compatibility with existing callers. - validateConfigPatch.js now imports from ./configValidation.js directly. * perf: split path string once in validateConfigPatchBody path.split('.') was called twice — once to extract topLevelKey and again for segments. Moved the single split to the top and derived topLevelKey from segments[0], avoiding the redundant allocation. * fix(#18): change lint script from tsc to biome check * fix(#19,#20): simplify params type; add PATCH body value check * fix(#21): add metadata export to config page * fix(#22): compute addedCount/removedCount inside useMemo loop * fix(#23,#24,#25,#26): tighten isGuildConfig; extract inputClasses; guard number inputs; rename DiscardChangesButton * fix(#27): change aria-live from polite to off on char counter * fix(#28): change Toaster theme from dark to system * fix(#29,#30): export BotApiConfig; return 504 on AbortError/TimeoutError * fix(#31): add one-time localStorage key migration from old key * fix(#32,#33,#34): SpamConfig JSDoc; collapse WritableConfigSection; fix SYSTEM_PROMPT_MAX_LENGTH JSDoc * fix: remove unused validateSingleValue import, fix biome formatting in config.js After the refactor, validateSingleValue is re-exported directly via 'export { } from' and no longer needs a local import binding. Also removed an extra blank line that biome flagged as a format error. * fix: add DNS resolution validation to prevent SSRF via DNS rebinding Add async validateDnsResolution() that resolves a webhook hostname via DNS and checks all resolved IPs against the existing blocked ranges before fetch. This closes the TOCTOU gap where a hostname could pass string-based validation then resolve to a private IP at request time (DNS rebinding attack). Changes: - Add validateDnsResolution() with resolve4/resolve6 checks - Integrate DNS check in fireAndForgetWebhook before fetch - Normalize IPv6 hostnames by stripping brackets (Node.js v22 retains them in URL.hostname, contrary to WHATWG spec) - Add comprehensive test coverage for DNS rebinding scenarios - Update webhook tests for async DNS validation flow Addresses CodeRabbit review thread #8. * fix: update test assertions for string[] return type from validateSingleValue * fix: mock validateDnsResolution in webhook integration tests After adding DNS resolution pinning in fireAndForgetWebhook, the config and guilds route tests need to mock validateDnsResolution to return true so fetch is actually called. * fix: address minor code review feedback - JSDoc, tests, caps * fix(frontend): address code review feedback - HTML, types, perf * fix(backend): address code review feedback - validation, logging, exports * fix: correct IPv6 validation for public addresses and literals * fix: restore classifier invocation in triage module * fix: address test failures in validateConfigPatch and triage-respond - Check for empty path segments before topLevelKey validation - Fix test to use valid nullable path (welcome.channelId) - Add mock cleanup between triage-respond tests * fix(validation): handle alertChannelId nullable and DNS edge cases * fix(security): prevent mask sentinel write-back and auth secret override 1. configAllowlist: Add isMasked() and stripMaskedWrites() to detect and filter out writes where sensitive fields contain the mask sentinel ('••••••••'). Prevents clients from accidentally overwriting real secrets with the placeholder returned by maskSensitiveFields(). 2. bot-api-proxy: Reorder header spread so x-api-secret is always set AFTER spreading options.headers, preventing caller-provided headers from overriding the server-side auth secret. Both fixes include comprehensive tests. * test: add missing test cases for mask sentinel, prototype pollution, DNS edge cases * refactor: simplify webhook validation for internal-only use * refactor: remove unused SSRF validation code Deleted validateWebhookUrl.js and its tests since webhooks are internal-only. Simplified webhook.js to just check URL format. * fix: prevent WebSearch notification failures from aborting response * fix: correct safeSend mock setup in triage-respond tests * fix(security): use own-property checks in config validation * fix: export MASK constant and clean up orphaned JSDoc * fix: report written sections in webhook, add roles endpoint test * fix: address remaining PR review feedback - Add nullable: true to triage.moderationLogChannel and debugFooterLevel - Add evalClient param to runResponder JSDoc - Convert SAFE_CONFIG_KEYS to Set for O(1) lookups - Reorder validation checks for consistent 400 responses - Update tests for Set-based SAFE_CONFIG_KEYS --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: AnExiledDev <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Frontend-backend contract fixes:
- MemberRow: id/displayName/avatar/messages_sent/warning_count/joinedAt (was snake_case)
- Avatar: use full URL from backend directly, remove hash-based CDN helper
- Pagination: cursor → nextAfter, param 'cursor' → 'after'
- MemberDetail: flat response shape with stats/reputation/warnings sub-objects
- SortColumn: restrict to API-supported values (messages/xp/warnings/joined)
- Role color: use hexColor string directly instead of number conversion
- XP progress: use next_level_xp from reputation object
- CSV export: add error state instead of silent catch
- Dependency array: add fetchMembers, remove eslint-disable comment
- Keyboard accessibility: tabIndex={0} + onKeyDown for Enter/Space on rows
- Guild context: handleRowClick depends on guildId
- Search total: display filteredTotal when available
Addresses review comments #1-7, #16-19 on PR #119.
* feat(dashboard): add member table component with sort and pagination * feat(dashboard): add members list page * feat(dashboard): add member detail page with stats and admin actions * feat(members): add member management API with detail, history, XP adjust, and CSV export * feat(members): mount members router, export guild middleware, remove superseded basic members endpoint * test(members): add API endpoint tests (26 tests) * fix: lint import ordering in member dashboard components * fix(members-api): add rate limiting, CSV injection protection, XP transaction safety - CSV injection: escapeCsv now prefixes formula chars (=,+,-,@,\t,\r) with single quote - XP bounds: cap adjustments to ±1,000,000 - XP transaction: wrap upsert + level update in BEGIN/COMMIT/ROLLBACK - Warning filter: add AND action = 'warn' to recent warnings query in member detail - CSV export: paginate guild.members.list() for guilds > 1000 members - Pool safety: wrap getPool() in safeGetPool() with try/catch, return 503 on failure - Search total: return filteredTotal alongside total when search is active Addresses review comments #8-15 on PR #119. * fix(dashboard): align member interfaces with backend API response shape Frontend-backend contract fixes: - MemberRow: id/displayName/avatar/messages_sent/warning_count/joinedAt (was snake_case) - Avatar: use full URL from backend directly, remove hash-based CDN helper - Pagination: cursor → nextAfter, param 'cursor' → 'after' - MemberDetail: flat response shape with stats/reputation/warnings sub-objects - SortColumn: restrict to API-supported values (messages/xp/warnings/joined) - Role color: use hexColor string directly instead of number conversion - XP progress: use next_level_xp from reputation object - CSV export: add error state instead of silent catch - Dependency array: add fetchMembers, remove eslint-disable comment - Keyboard accessibility: tabIndex={0} + onKeyDown for Enter/Space on rows - Guild context: handleRowClick depends on guildId - Search total: display filteredTotal when available Addresses review comments #1-7, #16-19 on PR #119. * feat(dashboard): add Next.js API proxy routes for member endpoints Create proxy routes following the existing bot-api-proxy pattern: - GET /api/guilds/:guildId/members — enriched member list - GET /api/guilds/:guildId/members/:userId — member detail - POST /api/guilds/:guildId/members/:userId/xp — XP adjustment - GET /api/guilds/:guildId/members/:userId/cases — mod case history - GET /api/guilds/:guildId/members/export — CSV export (streamed) All routes include guild admin authorization and proper error handling. CSV export uses 30s timeout and streams the response body. Addresses review comment #5 on PR #119. * test(members): update tests for API changes - XP tests: mock pool.connect() + client for transaction flow - Add XP bounds test (±1,000,000 limit) - Verify BEGIN/COMMIT/release called in transaction - Search test: assert filteredTotal in response * fix: lint and formatting fixes across all changed files - Use template literals instead of string concatenation (biome) - Use const for non-reassigned variables - Add button type='button' for a11y - Remove unused displayTotal variable - Use Number.isNaN over global isNaN - Format proxy routes to match biome standards * fix(members-api): add global + per-route rate limiting to satisfy CodeQL * 📝 Add docstrings to `feat/member-management` Docstrings generation was requested by @BillChirico. The following files were modified: * `src/api/routes/guilds.js` * `src/api/routes/members.js` * `web/src/app/api/guilds/[guildId]/members/[userId]/cases/route.ts` * `web/src/app/api/guilds/[guildId]/members/[userId]/route.ts` * `web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts` * `web/src/app/api/guilds/[guildId]/members/export/route.ts` * `web/src/app/api/guilds/[guildId]/members/route.ts` * `web/src/app/dashboard/members/[userId]/page.tsx` * `web/src/app/dashboard/members/page.tsx` * `web/src/components/dashboard/member-table.tsx` These files were ignored: * `tests/api/routes/guilds.test.js` * `tests/api/routes/members.test.js` * fix: correct CSV formula-injection bug in escapeCsv The escapeCsv function was discarding the original string value when prefixing with a single quote to neutralize formula injection. Now correctly preserves the value: str = `'${str}` instead of str = `'`. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: include guildId in member row click navigation Include guildId as a query parameter when navigating to member detail page to ensure guild context is preserved across navigation. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use safeGetPool in all member endpoints The GET /:id/members endpoint was using raw getPool() instead of safeGetPool() with the 503 guard used by all other member endpoints. Now consistently returns 503 when database is unavailable. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: log CSV export errors instead of silent failure Add console.error logging when CSV export fails, in addition to setting the error state for user display. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(members): dedupe rate limiting, add 401 handling, fix loading state, remove console.error * fix(members): reject fractional XP amounts (column is INTEGER) * test: boost branch coverage to 85% with targeted tests Raise branch coverage from 83.32% to 85.24% across 4 key files: - sentry.js: beforeSend filter, tracesSampleRate parsing, environment resolution - events.js: review/showcase/challenge button handlers, partial fetch, rate limit/link filter branches - rateLimit.js: repeat offender edge cases, permission checks, alert channel, cleanup - members.js: safeGetPool null paths (503), transaction rollback, escapeCsv edge cases New files: tests/modules/events-extra.test.js Modified: tests/modules/rateLimit.test.js, tests/sentry.init.test.js Removed unused import: src/api/index.js * fix(members): reject non-integer XP amounts with 400 The reputation.xp column is INTEGER. Fractional values like 1.5 pass the existing finite/non-zero check but get silently truncated by PostgreSQL (admin adds 1.5, only 1 is stored). Add explicit Number.isInteger check after the existing guards, returning 400 with 'amount must be an integer'. * test(members): add test for fractional XP amount returning 400 Covers the new Number.isInteger guard — sending amount: 1.5 must return 400 with error 'amount must be an integer'. * fix(members): scope membersRateLimit to member routes only The global router.use(membersRateLimit) was applied to every request hitting this router, which is mounted at /api/v1/guilds before the guilds router. This accidentally rate-limited non-member guild endpoints (e.g. /guilds/:id/analytics). Remove the global router.use and add membersRateLimit explicitly on each of the five member route definitions (export, list, detail, cases, xp) so rate limiting is scoped correctly. * fix(members-ui): fix roleColorStyle fallback for hex alpha concatenation The caller appends '40' and '15' to the result for borderColor and backgroundColor. When the fallback was 'hsl(var(--muted-foreground))' this produced 'hsl(var(--muted-foreground))40' — not valid CSS — so roles with Discord's default color (#000000) got no border/background. Replace the HSL CSS-variable fallback with a plain hex grey (#6b7280) so appending hex alpha digits produces valid 8-digit hex colours. * fix: biome formatting in members.js * fix(members): add AbortController and request sequencing to prevent stale responses Add AbortController to cancel in-flight fetch requests when a new request is triggered (e.g., from search/sort changes). Also add a monotonic request ID counter to guard against out-of-order responses overwriting newer state. - Abort previous request on each new fetchMembers call - Pass AbortSignal to fetch() - Silently ignore AbortError exceptions - Discard stale responses/errors via requestId guard - Only clear loading state for the current (non-superseded) request - Abort in-flight request on component unmount * fix(members): use Discord server-side search instead of page-local filtering Replace client-side substring filtering (which only searched within the current page of Discord results) with Discord's guild.members.search() API for server-side search across all guild members. - search param now triggers guild.members.search({ query, limit }) - Cursor pagination (nextAfter) is null during search (Discord search does not support cursor-based paging) - filteredTotal reflects actual server-side search result count - Sort still applies to the returned page (documented limitation — global sort would require DB-driven member indexing) - Updated tests to verify search() is called and filteredTotal semantics * fix(members): stream CSV export in batches to reduce memory pressure Replace the pattern of accumulating all guild members in one in-memory Map before writing CSV. Now each batch of 1000 members is fetched from Discord, enriched from the DB, written to the response, and released for GC — keeping peak memory at O(batch_size) instead of O(total_members). - Move CSV header write before the batch loop - Process and write each batch inline instead of collecting all first - Remove userIds.length > 0 guards (batch loop guarantees non-empty) - Track exportedCount incrementally - Added streaming export test * fix: button types, useCallback deps, array keys, remove duplicate tests and eslint comments --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <[email protected]>
1 participant
Summary
Complete repository infrastructure setup for bills-bot:
.editorconfigsrc/deploy-commands.jsfor slash command deploymentChanges
Linting & formatting
@biomejs/biomebiome.jsonlint,lint:fix,formatCI/CD
.github/workflows/ci.yml.github/workflows/claude-review.ymlDocumentation
README.md(setup, architecture, config reference, env vars, dev/deploy)AGENTS.mdCLAUDE.mdCONTRIBUTING.mdRepo hygiene
.editorconfigvitest+vitest.config.jstests/config.test.jstests/commands.test.jspackageManageris set ([email protected]).env.exampleto include required vars:DISCORD_TOKENDISCORD_CLIENT_IDOPENCLAW_API_KEYOPENCLAW_API_URLDATABASE_URLGUILD_ID,LOG_LEVELVerification
pnpm lint pnpm testNotes
CLIENT_ID,OPENCLAW_URL,OPENCLAW_TOKEN) while standard names are now primary.