feat: PostgreSQL-backed config persistence#14
Merged
BillChirico merged 27 commits intomainfrom Feb 11, 2026
Merged
Conversation
- Add src/db.js: PostgreSQL connection pool with auto-schema creation - Refactor src/modules/config.js: load/save config from DB with config.json seed - Extend /config command with 'set' and 'reset' subcommands - Add autocomplete for config paths in /config set - Graceful fallback to config.json when DATABASE_URL is not set - In-place config mutation ensures live propagation to all modules - Add pg dependency Config is stored as JSONB per section in a 'config' table. On first boot with an empty DB, seeds from config.json. Changes via /config set take effect immediately without restart.
Contributor
|
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 (2)
📝 WalkthroughWalkthroughAdds PostgreSQL support and DB-backed configuration with file fallback; refactors the /config command into view/set/reset with autocomplete and truncation; converts startup to an asynchronous flow that initializes DB, loads config/state, enhances interaction handling (including autocomplete and permission checks), and exposes pending-request and DB lifecycle helpers. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@src/commands/config.js`:
- Line 9: VALID_SECTIONS is duplicated across the file; derive the command
choices from it to avoid drift by creating a SECTION_CHOICES array (mapping
VALID_SECTIONS to objects with capitalized name and value) and then replace the
literal arrays passed to .addChoices(...) in both subcommands with
.addChoices(...SECTION_CHOICES); update any references to the old literal choice
arrays to use SECTION_CHOICES and keep the VALID_SECTIONS constant as the single
source of truth.
- Around line 82-88: The autocomplete currently only captures one nesting level
when building paths in the loop in src/commands/config.js (the
Object.entries/config loop) so deep keys like ai.providers.openai aren’t
suggested; add a recursive helper (e.g., flattenConfigKeys or collectPaths) that
walks nested objects and emits full dot-paths for each leaf, call it when
iterating over each section instead of only using Object.keys(value), and ensure
its output is pushed into the paths array so autocomplete matches
setConfigValue’s dot-notation support.
- Around line 148-161: The current loop that builds the embed (using
embed.setDescription, iterating over config and calling embed.addFields) can
produce an embed payload exceeding Discord's 6000-character limit; update the
logic to compute the cumulative embed content length while serializing each
config section and if the running total would exceed 6000 either (a) batch
fields into multiple Embed objects (create additional embeds and add them to the
reply, ensuring each embed's length <=6000 and not exceeding Discord's embed
count limit) or (b) switch to a condensed fallback that only lists section names
and prompts the user to call a new `/config view section:<name>` command; apply
this change around the loop that uses Object.entries(config) and embed.addFields
so long outputs are split or truncated at the embed level rather than only
per-field.
- Around line 197-200: The "New Value" field currently uses
path.split('.').slice(1)[0] which only grabs the first key after the section and
fails for deeper paths; change the logic where .addFields builds the New Value
(using the existing variables path, updatedSection, and value) to compute the
actual leaf value by splitting path on '.', removing the section segment, and
iteratively descending into updatedSection (e.g., with a reduce over the path
parts) to obtain the final value, falling back to the original value if the
descent yields undefined, and then use that computed leaf value in the New Value
field.
In `@src/db.js`:
- Around line 24-31: The current Pool creation sets ssl to rejectUnauthorized:
false for all non-Railway connections; change this to use a configurable flag
(e.g., process.env.PG_SSL_REJECT_UNAUTHORIZED) or a boolean derived from an env
var so the default is true and only explicitly disables certificate verification
when opted-in; update the ssl assignment in the new Pool(...) block (where pool
is constructed and ssl is set based on connectionString) to read the env var,
coerce it to a boolean, and use { rejectUnauthorized: <env-derived-boolean> }
for non-Railway connections.
- Around line 24-31: The Pool instantiation (variable pool created via new Pool)
lacks a listener for the pool 'error' event; add a pool.on('error', ...) handler
after the Pool is created (near the Pool constructor) that logs the error via
your logger (or console.error) and performs appropriate shutdown/recovery (e.g.,
graceful process exit or retry logic) to prevent unhandled 'error' events from
crashing the process; ensure the handler references the pool variable and
includes the error object in the log message for debugging.
In `@src/index.js`:
- Around line 250-252: The shutdown sequence must not be aborted if closeDb()
throws; wrap the await closeDb() call in a try/catch (inside the shutdown logic
where closeDb() is called) to catch and log any error from closeDb() and allow
execution to continue to client.destroy() and process.exit(0); ensure you
reference the existing process logger (or info/error functions used nearby) when
logging the caught error so the rest of the shutdown (client.destroy() and
process.exit(0)) always runs even if closeDb() fails.
In `@src/modules/config.js`:
- Around line 57-67: The seeding loop that inserts each key from fileConfig and
the reset-all loop perform multiple sequential writes without a transaction;
wrap both operations in a DB transaction so they are atomic: begin a transaction
before the for...of insert loop in the seeding logic (the block that iterates
over Object.entries(fileConfig) and then sets configCache = { ...fileConfig })
and commit after all inserts succeed, and do the same around the reset-all
sequence (the code that updates/deletes config entries in the reset-all path);
ensure you rollback on any error and only update configCache after a successful
commit.
- Around line 134-139: setConfigValue and resetConfig call getPool() unguarded
and will throw when DATABASE is not configured; wrap the pool acquisition and
query in a try/catch (or check for a valid pool) in both setConfigValue and
resetConfig so that if getPool() fails you still update the in-memory
configCache[section] (and updated_at) and skip the DB query, and surface a clear
user-facing/log message indicating the DB is unavailable and changes are
persisted only in memory (or alternatively, throw a controlled error noting the
command requires a DB). Ensure you reference and modify setConfigValue,
resetConfig, getPool, and configCache to implement the conditional DB write and
error handling.
- Around line 180-185: The code is spreading potentially primitive config values
which turns them into empty objects; update the logic that writes into
configCache (where you use configCache[key], value and where you used
fileConfig[section]) to first check whether the source (value or
fileConfig[section]) is a plain object (e.g., typeof === 'object' && value !==
null && !Array.isArray(value)) and only use Object.assign / object spread when
it is an object; otherwise assign the primitive directly (configCache[key] =
value) so primitives are preserved. Ensure the branch that clears existing
object entries (the for...delete and Object.assign block) still only runs when
configCache[key] is an object and the incoming value is an object.
- Around line 114-115: The variable sectionConfig is assigned but never used;
remove the dead assignment to clean up src/modules/config.js by deleting the
line that creates sectionConfig (const sectionConfig = {
...(configCache[section] || {}) };) and ensure any legitimate logic that
intended to use it instead references configCache[section] or the local variable
section where needed (check nearby uses of parts and configCache to confirm no
other changes are required).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
package.jsonsrc/commands/config.jssrc/db.jssrc/index.jssrc/modules/config.js
🧰 Additional context used
🧠 Learnings (2)
📚 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
🧬 Code graph analysis (4)
src/db.js (2)
src/modules/config.js (4)
pool(44-44)pool(55-55)pool(135-135)pool(152-152)src/logger.js (1)
info(218-220)
src/index.js (6)
src/commands/config.js (2)
config(79-79)config(124-124)src/logger.js (3)
config(27-27)error(232-234)info(218-220)src/db.js (3)
client(34-34)closeDb(70-76)initDb(18-53)src/modules/config.js (1)
loadConfig(40-82)src/modules/events.js (1)
registerEventHandlers(144-149)src/commands/status.js (1)
healthMonitor(58-58)
src/commands/config.js (1)
src/modules/config.js (5)
getConfig(88-90)section(114-114)sectionData(165-165)setConfigValue(108-143)resetConfig(150-191)
src/modules/config.js (2)
src/db.js (2)
pool(12-12)getPool(60-65)src/logger.js (1)
info(218-220)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (3)
package.json (1)
16-16: LGTM! Thepgdependency is necessary for the new PostgreSQL-backed configuration persistence feature.src/modules/config.js (1)
198-222:parseValue— reasonable helper, but be aware of edge cases.A few notes:
- The number regex (line 209) accepts strings like
""— actually no, the regex requires at least one digit. But it does accept leading-zero numbers like"007"which become7. Also, very large numbers will lose precision (beyondNumber.MAX_SAFE_INTEGER).- A string
"[not json"that starts with[but isn't valid JSON correctly falls through to plain string — good.Overall this is fine for a Discord bot config use case.
src/index.js (1)
291-327: Async startup sequence looks well-structured.The ordering (DB init → config load → state load → event registration → commands → login) correctly ensures dependencies are available before consumers. The conditional DB initialization and the fallback in
loadConfigprovide a smooth degradation path for environments withoutDATABASE_URL.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
BillChirico
force-pushed
the
feat/db-config
branch
from
e42ff42 to
efb4a87
Compare
- db.js: Make SSL rejectUnauthorized configurable via DB_SSL_REJECT_UNAUTHORIZED env var (default: true)
- db.js: Add pool.on('error') handler to prevent unhandled errors crashing the process
- config.js (module): Write to DB first, then update cache (fixes inconsistent state on DB failure)
- config.js (module): Wrap DB seeding and reset-all in transactions for atomicity
- config.js (module): Add exitOnError param to loadConfigFromFile (prevents process.exit at runtime)
- config.js (module): Guard setConfigValue/resetConfig against missing DB with graceful fallback
- config.js (module): Remove unused sectionConfig variable and getConfigSection export
- config.js (module): Fix spreading primitive values (use isPlainObject guard)
- config.js (command): Derive SECTION_CHOICES from VALID_SECTIONS (single source of truth)
- config.js (command): Recursive flattenConfigKeys for deep autocomplete paths
- config.js (command): Add embed character limit guard with truncation notice
- config.js (command): Fix New Value display for nested paths using reduce traversal
- index.js: Try/catch closeDb() in shutdown to prevent blocking remaining cleanup
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/commands/config.js`:
- Around line 161-188: The local variable truncated is unused after being set in
the loop; remove the truncated declaration and the assignment truncated = true
(and keep the existing embed.addFields truncation notice), or alternatively use
truncated after the loop to modify the embed footer/message (e.g., call
embed.setFooter or append to embed.data.footer) to indicate truncation—update
references to truncated, the for-loop over Object.entries(config), and the
truncation branch that checks totalLength + fieldLength > EMBED_CHAR_LIMIT - 200
accordingly so there are no unused variables left.
- Around line 144-157: The single-section embed path in src/commands/config.js
currently passes the raw JSON for a section into embed.addFields (the block
using sectionData) which can exceed Discord's 1024 char field limit and cause
interaction.reply to throw; change this by serializing sectionData to a jsonStr
(JSON.stringify(sectionData, null, 2)), truncate jsonStr to ~1000 characters
(e.g., if jsonStr.length > 1000 use jsonStr.slice(0, 997) + '...'), then pass
the truncated string inside the fenced '```json\n' + truncatedJson + '\n```' as
the field value in the embed.addFields call so the single-section view matches
the length-guard used in the full-view path.
In `@src/db.js`:
- Around line 69-74: The initDb error path can leave the shared variable pool
non-null which makes getPool() return an unusable pool; update initDb to
defensively reset/release pool on any failure (e.g., in the catch block set pool
= null and close the client/pool if created) before rethrowing the error so
getPool() will throw as expected; reference the initDb function, the shared pool
variable, and getPool to locate where to null out/cleanup the pool.
In `@src/index.js`:
- Around line 39-40: Add a brief inline comment where the top-level variable
"config" is initialized (the let config = {} declaration) explaining that this
variable is later assigned to the object returned by loadConfig(), which is the
same shared object as configCache in src/modules/config.js, and that in-place
mutations via setConfigValue and resetConfig intentionally propagate through
this shared reference so the top-level config remains in sync across modules.
In `@src/modules/config.js`:
- Around line 131-141: The code calls parseValue(value) twice when updating
nested keys (once when setting current[finalKey] in the sectionClone and again
when updating the in-memory cache), causing redundant work; compute const parsed
= parseValue(value) once and use parsed for both assignments (the clone
assignment where current[finalKey] is set and the later cache update logic that
writes the same key), updating references to parseValue in the functions that
handle sectionClone, parts, finalKey and the cache update to use parsed instead.
- Around line 24-44: Replace brittle string comparison in loadConfigFromFile by
marking the "config not found" error with a stable flag/property and checking
that flag when deciding to re-throw; specifically, when you create the not-found
error inside loadConfigFromFile (current msg 'config.json not found!'), attach a
property such as err.code = 'CONFIG_NOT_FOUND' (or throw a custom
ConfigNotFoundError) and then in the catch block test that property (e.g.,
err.code === 'CONFIG_NOT_FOUND' or instanceof ConfigNotFoundError) instead of
matching err.message so the re-throw behavior remains correct even if the
message text changes.
- Around line 51-63: loadConfig currently calls loadConfigFromFile() with the
default exit-on-error behavior which causes the process to exit if config.json
is missing before checking the DB; change loadConfig to call
loadConfigFromFile({ exitOnError: false }) so a missing file becomes non-fatal,
then attempt getPool() and only treat missing configuration as fatal if both the
file (now optional) and the DB (getPool()/DB config read) are unavailable;
update logic around loadConfig, getPool, and configCache to prefer DB values
when present and fall back to fileConfig only when DB is not initialized.
- Around line 120-171: setConfigValue performs a read-modify-write of the entire
section (using structuredClone and INSERT ... ON CONFLICT) which allows
concurrent calls to clobber each other; modify setConfigValue to perform an
atomic partial update at the DB level (or acquire a DB advisory lock around the
section) instead of writing the whole section: use getPool() to run an UPDATE
that leverages jsonb_set (or jsonb ||) to set the nested key inside the stored
JSON value for the given section key, falling back to the in-memory mutation of
configCache only after the DB update succeeds, and preserve current parseValue,
configCache, and logging behavior (references: setConfigValue, configCache,
getPool, parseValue).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/commands/config.jssrc/db.jssrc/index.jssrc/modules/config.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/db.js (2)
src/modules/config.js (6)
pool(55-55)pool(66-66)pool(146-146)pool(181-181)client(71-71)client(214-214)src/logger.js (1)
info(218-220)
src/modules/config.js (2)
src/db.js (3)
pool(12-12)getPool(69-74)client(43-43)src/logger.js (1)
info(218-220)
src/commands/config.js (1)
src/modules/config.js (4)
getConfig(109-111)section(126-126)setConfigValue(120-171)resetConfig(178-245)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (4)
src/index.js (2)
162-175: Autocomplete handler looks good.Properly guards against missing
autocompleteexport with optional chaining, catches errors, and returns early to avoid falling through to the command handler.
304-326: Startup sequence is well-ordered.DB init → config load → state load → event registration → command load → login is the correct dependency order. The conditional DB init (Line 306) and the
loadConfig()fallback design ensure the bot works withoutDATABASE_URL.One minor note: if
loadConfig()throws (e.g.,config.jsonmissing and DB unavailable),startup().catchat Line 328 will log and exit — which is the right behavior.src/modules/config.js (2)
252-254:isPlainObjecthelper is clean and correctly handles null/array edge cases.
261-285:parseValueis straightforward and covers common Discord command input types.One edge case to be aware of: strings like
"0123"will be coerced to the number123. If octal-like prefixed strings are possible config values, you may want to add a leading-zero guard. Unlikely to matter for bot config in practice.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Truncate single-section field values to 1024-char Discord limit - Use `truncated` flag to adjust embed footer when sections omitted - Null out pool and call pool.end() if initDb() fails after creation - Add inline comment explaining config shared reference coupling - Use err.code = 'CONFIG_NOT_FOUND' instead of brittle string matching - loadConfig() no longer hard-exits on missing config.json if DB available - Use jsonb_set() for atomic partial DB updates in setConfigValue() - Store parseValue() result in parsedVal to avoid redundant calls
Collaborator
Author
✅ Round 2 Review Comments AddressedAll 8 CodeRabbit review comments have been fixed in commit Changes
All 10 unresolved review threads have been resolved. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Remove process.exit from loadConfigFromFile — always throw instead (no runtime code path needs hard exit; callers already handle errors) - Fix parseValue to preserve large numeric strings beyond MAX_SAFE_INTEGER instead of silently corrupting precision - Replace jsonb_set with transaction + row lock for config writes: fixes deep nested paths where intermediate keys don't exist in DB, and prevents concurrent section writes from clobbering each other - Remove unused subPath variable (dead store after DB write refactor)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/commands/config.js`:
- Around line 138-142: The embed footer hardcodes "Stored in PostgreSQL…"
causing incorrect text when env DATABASE_URL is not set; update the footer
generation where EmbedBuilder is instantiated (the const embed = new
EmbedBuilder() block) to compute the footer text dynamically by checking
process.env.DATABASE_URL (or the config-check helper used in this module) and
set it to "Stored in PostgreSQL • Use /config set to modify" when present and
"Stored in file • Use /config set to modify" (or similar) when absent, then pass
that string into setFooter so the message reflects the actual storage backend.
In `@src/db.js`:
- Around line 18-70: initDb() can leak the previous Pool when called twice
because the global pool is overwritten; update initDb to check the existing pool
variable at the start and either return it immediately if already
initialized/healthy or gracefully close it (await pool.end()) before creating a
new Pool, ensuring you handle errors from pool.end() and set pool = null when
closed; reference the initDb function and the module-level pool variable so the
guard runs before new Pool construction and before any operations that assume a
fresh pool.
In `@src/index.js`:
- Around line 307-334: The startup function logs "Database initialized" after
calling initDb(), which is redundant because initDb() already logs "Database
connected" and "Database schema initialized"; remove or alter the extra log in
startup (the info('Database initialized') call inside async function startup) to
avoid noisy duplicate messages—either delete that info() call or replace it with
a more specific message that adds unique context.
In `@src/modules/config.js`:
- Around line 86-88: The catch block that currently does await
client.query('ROLLBACK') can itself throw and mask the original txErr; modify
the error handling around the rollback so the rollback is performed inside its
own try/catch (or use a safe helper) and any rollback error is logged/suppressed
while rethrowing the original txErr; specifically update the catch that
references txErr and client.query('ROLLBACK') to call rollback inside try {
await client.query('ROLLBACK') } catch (rbErr) { /* log rbErr */ } and then
throw txErr to preserve the original error.
- Around line 266-270: In resetConfig, guard the ROLLBACK call so a failed
rollback doesn't replace the original transaction error: when catching txErr,
call client.query('ROLLBACK') with a `.catch(() => {})` (same approach used in
setConfigValue) to swallow any rollback errors, then rethrow the original txErr;
reference the resetConfig function and the client.query('ROLLBACK') invocation
to locate where to apply this change.
- Around line 254-284: The reset-all path currently upserts keys from fileConfig
but never removes DB or in-memory keys that were added at runtime; update the
block that runs when pool exists to, inside the same transaction (BEGIN/COMMIT),
delete any rows in config whose key is NOT IN the set of Object.keys(fileConfig)
(use a parameterized DELETE ... WHERE key NOT IN (...)) before
inserting/updating the fileConfig entries, and likewise update the in-memory
mutation on configCache to remove any keys not present in fileConfig (e.g.,
iterate Object.keys(configCache) and delete keys absent from fileConfig before
doing the existing merge/assign logic). Ensure you use the same client
(client.query) for the DELETE and keep the rollback on error behavior intact.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/commands/config.jssrc/db.jssrc/index.jssrc/modules/config.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/commands/config.js (1)
src/modules/config.js (5)
getConfig(116-118)section(133-133)err(25-25)setConfigValue(127-214)resetConfig(221-288)
src/db.js (2)
src/modules/config.js (4)
pool(52-52)pool(66-66)pool(154-154)pool(224-224)src/logger.js (1)
info(218-220)
src/modules/config.js (2)
src/logger.js (1)
info(218-220)src/db.js (2)
pool(12-12)getPool(77-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (15)
src/db.js (2)
24-40: Past review items addressed — SSL config and pool error handler look good.SSL handling is now environment-driven via
DB_SSL_REJECT_UNAUTHORIZED, and the pool'error'event handler prevents unhandled crashes. The try/catch cleanup at Lines 62-67 properly nulls the pool on initialization failure.
77-93:getPool()andcloseDb()are clean and correct.src/commands/config.js (5)
9-15:SECTION_CHOICESderived fromVALID_SECTIONS— clean single source of truth.The camelCase-to-spaced regex on Line 13 is a nice touch for display names.
65-105:flattenConfigKeysandautocompletelook correct.The recursive flattening correctly handles arbitrary nesting, includes intermediate paths (useful for setting sub-objects), and the autocomplete respects Discord's 25-result limit.
159-193: Full-view truncation and footer adjustment look correct.The cumulative size tracking against
EMBED_CHAR_LIMIT, per-field truncation to 1000 chars, truncation notice, and conditional footer update all address the prior review feedback properly.
208-248:handleSet— solid implementation with proper defer/error handling.Section validation before deferReply, leaf value traversal for display, and the deferred-aware catch block are all correct.
250-281:handleResetlooks good.The
section || undefinedcoercion correctly routes to the reset-all path when no section is provided.src/index.js (3)
39-43: Shared-reference comment is clear and helpful.Good documentation of the non-obvious coupling between the module-level
configandconfigCacheinmodules/config.js.
165-178: Autocomplete routing is correct and well-guarded.The early return on Line 177 ensures autocomplete interactions don't fall through to the command handler. Error handling wraps the call properly.
253-259:closeDb()properly guarded in shutdown — addresses prior review feedback.src/modules/config.js (5)
23-34:loadConfigFromFile— clean implementation witherr.codeflag.The
CONFIG_NOT_FOUNDerror code replaces the brittle string matching from the prior review. Clean separation of "missing file" vs "parse error".
41-110:loadConfig— well-structured DB-first with file fallback.The layered error handling (file optional → DB unavailable check → DB empty seed → DB failure fallback) is thorough and correctly addresses all prior review feedback. Transaction-wrapped seeding prevents partial writes.
127-214:setConfigValue— transaction + row lock is a solid approach.The
SELECT ... FOR UPDATEpattern correctly prevents concurrent write clobbering, and the in-memory cache update mirrors the DB write logic. TheparsedValis computed once and reused. TheROLLBACKon Line 190 is properly guarded with.catch(() => {}).
304-333:parseValuehandles edge cases well.Safe integer range preservation for large numbers (e.g., Discord snowflake IDs), JSON structure detection with fallback, and clean boolean/null parsing. Solid implementation.
290-297:isPlainObjectis correct and addresses the prior review about spreading primitives.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Footer previously assumed PostgreSQL was always in use. Now shows 'Stored in PostgreSQL' when DATABASE_URL is set, or 'Stored in memory (config.json)' when running without a database.
If initDb() is called more than once, the previous pool is now gracefully closed before creating a new one, preventing connection leaks.
…rror Wrap ROLLBACK in try/catch so a rollback failure doesn't replace the original transaction error.
Reset-all now deletes DB rows and cache entries for keys that exist in the database but not in config.json, ensuring a clean slate.
Use explicit try/catch instead of .catch(() => {}) to guard the
ROLLBACK call, matching the pattern used in other transaction blocks.
Collaborator
Author
✅ Review Comments AddressedAll 6 review threads have been resolved:
|
This comment has been minimized.
This comment has been minimized.
Add validation that rejects __proto__, constructor, and prototype as path segments in setConfigValue. The check runs before any traversal loop, protecting the sectionClone, DB merge, and in-memory cache paths from prototype pollution attacks.
flattenConfigKeys was pushing both intermediate object nodes and their children. Selecting an intermediate path and setting a scalar value would destroy all nested config beneath it. Now only leaf paths (non- object values) are included in autocomplete suggestions.
Replace hardcoded VALID_SECTIONS array with Object.keys(loadConfigFromFile()) so static slash-command choices stay in sync with config.json automatically. handleSet validation now uses live config keys to also accept DB-added sections.
In file-only deployments (no DATABASE_URL), the DB fallback path in setConfigValue is expected behavior, not an error. Use logWarn instead of logError to avoid noisy error-level logging for normal operation.
Mirrors the existing guard in setNestedValue. If a section root is an array, replace it with a plain object before attempting nested writes to prevent silent data corruption.
Same rationale as setConfigValue — in file-only deployments the DB fallback is expected behavior, not an error condition.
Collaborator
Author
✅ Round 10 — All 4 review threads addressedCommits pushed to
|
| import { getConfig, setConfigValue, resetConfig, loadConfigFromFile } from '../modules/config.js'; | ||
|
|
||
| // Derived from config.json top-level keys so static slash-command choices stay in sync automatically. | ||
| const VALID_SECTIONS = Object.keys(loadConfigFromFile()); |
There was a problem hiding this comment.
Module-level loadConfigFromFile() breaks DB-only deployments
High Severity
VALID_SECTIONS is computed by calling loadConfigFromFile() at module-import time, which throws if config.json is absent. In DB-only deployments (after initial seeding), this prevents the entire /config command from loading — the very command needed to manage configuration remotely. Additionally, since loadCommands() runs after loadConfig() in the startup sequence, configCache is already populated from the DB at import time, so getConfig() would be the correct source and would also reflect any DB-only sections that config.json doesn't contain.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
Or push these changes by commenting: |
BillChirico
added a commit
that referenced
this pull request
Feb 11, 2026
Rebased PR #15 changes on top of main (post PR #14 merge). Deep config path autocomplete: - Nested key traversal with dot-notation (e.g. ai.model, welcome.enabled) - Array-aware setNestedValue with numeric index support - Dynamic section autocomplete from live config (not static choices) - collectConfigPaths handles arrays, empty objects/arrays Dynamic welcome messages: - Community snapshots with member/channel/voice activity stats - GuildVoiceStates intent for voice channel tracking - Welcome message module with configurable templates Review fixes (Rounds 1-10): - structuredClone for safe config cloning (no shared refs) - Array.isArray guards in section reset and cache updates - Channel validity filtering - Section name sanitization (escapeInlineCode) - INSERT ON CONFLICT for concurrent section inserts - resetConfig error handling with graceful DB fallback - Re-entrancy guard in initDb - getSslConfig helper with DATABASE_SSL env var support - fileConfigCache for single config.json load - Stale timestamp pruning in getCommunitySnapshot
BillChirico
added a commit
that referenced
this pull request
Feb 11, 2026
Rebased PR #15 changes on top of main (post PR #14 merge). Deep config path autocomplete: - Nested key traversal with dot-notation (e.g. ai.model, welcome.enabled) - Array-aware setNestedValue with numeric index support - Dynamic section autocomplete from live config (not static choices) - collectConfigPaths handles arrays, empty objects/arrays Dynamic welcome messages: - Community snapshots with member/channel/voice activity stats - GuildVoiceStates intent for voice channel tracking - Welcome message module with configurable templates Review fixes (Rounds 1-10): - structuredClone for safe config cloning (no shared refs) - Array.isArray guards in section reset and cache updates - Channel validity filtering - Section name sanitization (escapeInlineCode) - INSERT ON CONFLICT for concurrent section inserts - resetConfig error handling with graceful DB fallback - Re-entrancy guard in initDb - getSslConfig helper with DATABASE_SSL env var support - fileConfigCache for single config.json load - Stale timestamp pruning in getCommunitySnapshot
7 tasks
7 tasks
5 tasks
BillChirico
added a commit
that referenced
this pull request
Feb 17, 2026
- 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)
BillChirico
added a commit
that referenced
this pull request
Feb 17, 2026
- 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)
This was referenced Feb 23, 2026
BillChirico
added a commit
that referenced
this pull request
Feb 24, 2026
4 tasks
BillChirico
added a commit
that referenced
this pull request
Feb 25, 2026
… (#83) * feat(dashboard): add Discord entity pickers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * refactor: extract shared config allowlist, webhook utility, and proxy helpers; remove dead code Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * refactor(backend): extract shared validators and getBotOwnerIds, add webhook utility tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> Co-authored-by: AnExiledDev <AnExiledDev@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This was referenced Feb 25, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds PostgreSQL-backed configuration persistence so bot settings can be changed remotely via Discord slash commands without redeployment.
Changes
src/db.js— New PostgreSQL connection pool module with auto-schema creationsrc/modules/config.js— Refactored to load/save config from PostgreSQL withconfig.jsonas seed/fallbacksrc/commands/config.js— Extended with/config setand/config resetsubcommands + autocompletesrc/index.js— Async startup sequence: DB init → config load → event registration → loginpackage.json— AddedpgdependencyHow it works
DATABASE_URLset, seeds theconfigtable fromconfig.json/config set ai.model gpt-4oupdates both DB and in-memory config (no restart needed)/config reset [section]reverts toconfig.jsondefaultsDATABASE_URL, falls back gracefully to file-based configRailway Setup
Set
DATABASE_URLenvironment variable in Railway:Slash Commands
/config view [section]— View current config/config set <path> <value>— Set a value (e.g.,ai.model,welcome.enabled)/config reset [section]— Reset to config.json defaults