feat: add AuditLogManager for dashboard settings audit logging#188
Open
RedStar071 wants to merge 37 commits into
Open
feat: add AuditLogManager for dashboard settings audit logging#188RedStar071 wants to merge 37 commits into
RedStar071 wants to merge 37 commits into
Conversation
- Add AuditLog Prisma model with migration - Add AuditLogManager following PermissionNodeManager conventions - Add AuditLogChange type and PrismaJson augmentation - Integrate into SettingsContext with constructor/onPatch pattern - Wire audit logging into settings.patch.ts with structuredClone snapshot - Add GET /guilds/:guild/audit-logs endpoint with pagination - Add 18 unit tests for AuditLogManager
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements audit logging for guild settings and command execution: deterministic envelope canonicalisation and SHA‑256 hashing, hash‑chained storage in Postgres via Prisma, an AuditLogManager for serialised writes and channel emits, API retrieval, embed builders, listener hooks, tests, and a verification script. ChangesGuild Settings Audit Logging System
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Introduces dashboard settings audit logging by adding a new AuditLog Prisma model plus a settings-side AuditLogManager, wiring it into the settings PATCH flow, and exposing a paginated API endpoint to read audit log entries.
Changes:
- Add
AuditLogPrisma model + migration (tableaudit_log, index on(guild_id, created_at DESC)). - Add
AuditLogManagerand integrate it intoSettingsContext, plus exportedreadSettingsAuditLog()helper. - Add
GET /guilds/:guild/audit-logsendpoint and add audit-log writes toPATCH /guilds/:guild/settings, plus unit tests forAuditLogManager.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/lib/database/settings/structures/AuditLogManager.test.ts | Adds unit tests for audit log manager behavior (constructor/onPatch/update/add/remove + section/value derivation). |
| src/routes/guilds/[guild]/settings.patch.ts | Adds fire-and-forget audit logging on settings mutation (currently has a snapshot correctness issue). |
| src/routes/guilds/[guild]/audit-logs.get.ts | Adds paginated endpoint to fetch audit logs for a guild (minor consistency issue vs other routes). |
| src/lib/types/Augments.d.ts | Adds PrismaJson augmentation type AuditLogChanges. |
| src/lib/database/settings/types.ts | Introduces AuditLogChange type. |
| src/lib/database/settings/structures/AuditLogManager.ts | Implements AuditLogManager with update/add/remove/write + section/value helpers. |
| src/lib/database/settings/index.ts | Re-exports AuditLogManager. |
| src/lib/database/settings/functions.ts | Adds readSettingsAuditLog() helper (currently returns cached manager, affecting snapshot use-cases). |
| src/lib/database/settings/context/SettingsContext.ts | Adds auditLog manager to SettingsContext and updates it on patch. |
| prisma/schema.prisma | Adds AuditLog model and Guild.auditLogs relation. |
| prisma/migrations/20260416120000_add_dashboard_audit_log/migration.sql | Creates audit_log table, index, and FK to guilds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace AuditChainHead per-tenant design with singleton id=default - Rewrite migration.sql: UUID PK on audit_event, actorType/targetType columns, hash as UNIQUE (not PK), no requestId/traceId columns - Remove sequence: bigint from AuditEnvelopeInput (no lastSequence) - Rewrite AuditLogManager#write: global advisory lock, findUnique by id=default, correct auditEvent column set (actorType, no requestId) - Regenerate Prisma client (singleton AuditChainHead shape) - Fix tests: makeHead singleton shape, drop lastSequence assertions
- Replace auditLog.findMany with auditEvent.findMany filtered by
tenantId + action IN DASHBOARD_AUDIT_ACTIONS
- Rename take/skip -> limit/offset (parseInt, clamped to [1,100])
- Add patchToChanges helper: auditDiff {before,after} -> {added,removed,changed}
- Parallel member resolution with fallback for unresolved actors
- Return DashboardAuditEntry[] with id=UUID, guildId from route param
- settings.patch: capture before/after snapshots around submit() and
pass both to auditLog.update(actorId, before, after)
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/wolfstar-project/wolfstar/sessions/fdaf6952-6f06-465f-9108-8d00604a0b16 Co-authored-by: RedStar071 <76824516+RedStar071@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
prisma/schema.prisma (2)
246-252: Operational consideration: Global chain head serialization.The singleton chain head (
id='default') means all audit writes across all guilds serialize through one advisory lock. For low-frequency dashboard settings changes, this is acceptable. If audit logging expands to high-frequency events, consider per-tenant chain heads to reduce lock contention.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema.prisma` around lines 246 - 252, The AuditChainHead model uses a singleton id ("default") which serializes all tenants through one chain head; update the model to support per-tenant chain heads by adding a tenant identifier (e.g., guildId or tenantId String field), make the primary key include that tenant identifier (either by replacing the single id with guildId as `@id` or by introducing a composite @@id([guildId, id]) ), and ensure hash/updatedAt remain and map stays @@map("audit_chain_head"); adjust any code that references AuditChainHead.id to use the tenant-aware key (AuditChainHead.guildId/AuditChainHead.tenantId) so advisory locks can be taken per-tenant.
227-228: ⚡ Quick winConsider aligning ID column types with Guild model.
actorIdandtenantIduse unboundedTEXT, whilstGuild.idusesVARCHAR(19)(Discord snowflake size). Using consistent types would improve index efficiency and make the schema more self-documenting.♻️ Suggested schema alignment
- actorId String `@map`("actor_id") + actorId String `@map`("actor_id") `@db.VarChar`(19) ... - targetId String? `@map`("target_id") + targetId String? `@map`("target_id") `@db.VarChar`(19) ... - tenantId String? `@map`("tenant_id") + tenantId String? `@map`("tenant_id") `@db.VarChar`(19)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/schema.prisma` around lines 227 - 228, actorId and tenantId are currently mapped to unbounded TEXT while Guild.id uses VARCHAR(19); change the Prisma field definitions for actorId and tenantId to use the same fixed-length DB type as Guild.id (e.g., String with `@db.VarChar`(19)) so index size and semantics match; update the fields actorId and tenantId in the relevant model(s) to use the VarChar(19) mapping and keep existing `@map` annotations intact to preserve column names.src/lib/database/settings/structures/AuditLogManager.ts (1)
125-133: 💤 Low valueConsider defensive handling of non-JSON-safe values.
#toJsonSafehandlesbigint, arrays, and plain objects, but edge cases likeDateobjects, functions, or symbols would either serialize incorrectly or fail silently. For guild settings data, these edge cases are unlikely, but adding explicit guards would improve robustness.♻️ More defensive implementation
`#toJsonSafe`(value: unknown): unknown { + if (value === undefined) return null; if (typeof value === 'bigint') return value.toString(); + if (value instanceof Date) return value.toISOString(); if (Array.isArray(value)) return value.map((v) => this.#toJsonSafe(v)); if (value !== null && typeof value === 'object') { + if (Object.getPrototypeOf(value) !== Object.prototype) { + throw new TypeError(`Cannot serialize non-plain object: ${value.constructor.name}`); + } return Object.fromEntries(Object.entries(value as Record<string, unknown>).map(([k, v]) => [k, this.#toJsonSafe(v)])); } return value; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/database/settings/structures/AuditLogManager.ts` around lines 125 - 133, The `#toJsonSafe` method currently only handles bigint, arrays, and plain objects; update it to defensively handle Date (convert to ISO string), Map/Set (convert to plain object/array), symbol (convert to string via Symbol.prototype.toString or fallback), functions (replace with a descriptive placeholder like "[Function]"), and undefined (convert to null), and ensure it falls back to a safe serializable representation for any other non-plain object types to avoid silent failures when AuditLogManager serializes guild settings; keep the recursive structure and use Object.entries for objects and Array.from for iterables so existing callers of `#toJsonSafe` continue to work.prisma/migrations/20260416120000_add_dashboard_audit_log/migration.sql (1)
1-1: Migration data loss consideration.
DROP TABLE ... CASCADEdestroys existingaudit_logdata and dependent objects. For development iterations, this is acceptable. Before production deployment, ensure appropriate data migration or archival strategy if preserving audit history is required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prisma/migrations/20260416120000_add_dashboard_audit_log/migration.sql` at line 1, The migration currently issues a destructive DROP TABLE IF EXISTS "audit_log" CASCADE which will erase all audit_log rows and dependent objects; update the migration in migration.sql to preserve production data by replacing the unconditional DROP with a safe upgrade path: either (a) create a new table (or ALTER the existing one) and migrate rows from "audit_log" into it, or (b) RENAME the existing "audit_log" to an archival table before creating the new schema, and include an explicit data-copy/transform step and/or a backup table creation so dependent objects are preserved; ensure all steps reference "audit_log" and the DROP is removed or guarded for dev-only runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/database/settings/structures/AuditLogEnvelope.ts`:
- Around line 21-27: The canonicalize function fails to return a string for
undefined (JSON.stringify(undefined) yields undefined), breaking its return
contract and array joining; update canonicalize to explicitly handle undefined
(e.g., return JSON.stringify(null) or another stable string) before the
typeof/object checks so calls to canonicalize (and Array.map + join) always
yield strings; reference the canonicalize function and ensure the undefined
branch is placed before the "typeof value !== 'object'" check.
In `@src/routes/guilds/`[guild]/audit-logs.get.ts:
- Around line 100-103: The current code builds uniqueActorIds then calls
guild.members.fetch for every id (resolvedMembers) which hits the API even for
cached members; change to a cache-first approach: for each id in uniqueActorIds
check guild.members.cache.get(id) and use serializeMember on cached ones,
collect only the ids missing from cache, call guild.members.fetch only for those
missing ids (Promise.all), then merge fetch results with cached results into the
same memberMap (keep keys as id and values from serializeMember) so you avoid
unnecessary API fetches while preserving existing memberMap construction.
In `@src/routes/guilds/`[guild]/settings.patch.ts:
- Line 49: The call to auditLog.update(request.auth!.id, before, after)
currently swallows all errors; update this to preserve async behavior but
surface failures by attaching a .catch handler that logs the error along with
the guild and user context (use request.auth!.id and the guild identifier and/or
before/after summary) instead of returning null, so failures to write audit
entries are recorded for troubleshooting while keeping the call non-blocking.
---
Nitpick comments:
In `@prisma/migrations/20260416120000_add_dashboard_audit_log/migration.sql`:
- Line 1: The migration currently issues a destructive DROP TABLE IF EXISTS
"audit_log" CASCADE which will erase all audit_log rows and dependent objects;
update the migration in migration.sql to preserve production data by replacing
the unconditional DROP with a safe upgrade path: either (a) create a new table
(or ALTER the existing one) and migrate rows from "audit_log" into it, or (b)
RENAME the existing "audit_log" to an archival table before creating the new
schema, and include an explicit data-copy/transform step and/or a backup table
creation so dependent objects are preserved; ensure all steps reference
"audit_log" and the DROP is removed or guarded for dev-only runs.
In `@prisma/schema.prisma`:
- Around line 246-252: The AuditChainHead model uses a singleton id ("default")
which serializes all tenants through one chain head; update the model to support
per-tenant chain heads by adding a tenant identifier (e.g., guildId or tenantId
String field), make the primary key include that tenant identifier (either by
replacing the single id with guildId as `@id` or by introducing a composite
@@id([guildId, id]) ), and ensure hash/updatedAt remain and map stays
@@map("audit_chain_head"); adjust any code that references AuditChainHead.id to
use the tenant-aware key (AuditChainHead.guildId/AuditChainHead.tenantId) so
advisory locks can be taken per-tenant.
- Around line 227-228: actorId and tenantId are currently mapped to unbounded
TEXT while Guild.id uses VARCHAR(19); change the Prisma field definitions for
actorId and tenantId to use the same fixed-length DB type as Guild.id (e.g.,
String with `@db.VarChar`(19)) so index size and semantics match; update the
fields actorId and tenantId in the relevant model(s) to use the VarChar(19)
mapping and keep existing `@map` annotations intact to preserve column names.
In `@src/lib/database/settings/structures/AuditLogManager.ts`:
- Around line 125-133: The `#toJsonSafe` method currently only handles bigint,
arrays, and plain objects; update it to defensively handle Date (convert to ISO
string), Map/Set (convert to plain object/array), symbol (convert to string via
Symbol.prototype.toString or fallback), functions (replace with a descriptive
placeholder like "[Function]"), and undefined (convert to null), and ensure it
falls back to a safe serializable representation for any other non-plain object
types to avoid silent failures when AuditLogManager serializes guild settings;
keep the recursive structure and use Object.entries for objects and Array.from
for iterables so existing callers of `#toJsonSafe` continue to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 8e1d5469-57e2-4218-900b-a36b253772b2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
package.jsonprisma/migrations/20260416120000_add_dashboard_audit_log/migration.sqlprisma/schema.prismasrc/lib/database/settings/auditActions.tssrc/lib/database/settings/context/SettingsContext.tssrc/lib/database/settings/functions.tssrc/lib/database/settings/index.tssrc/lib/database/settings/structures/AuditLogEnvelope.tssrc/lib/database/settings/structures/AuditLogManager.tssrc/lib/database/settings/types.tssrc/lib/types/Augments.d.tssrc/routes/guilds/[guild]/audit-logs.get.tssrc/routes/guilds/[guild]/settings.patch.tstests/lib/database/settings/structures/AuditLogEnvelope.test.tstests/lib/database/settings/structures/AuditLogManager.test.ts
Contributor
Resolved in f7bf803. I merged in the current main-branch changes, fixed the package.json/pnpm-lock.yaml conflicts, and validated the result with lint, build, targeted settings-manager tests, and final review/security checks. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.coderabbit.yml (1)
21-23: 💤 Low valueVerify the slop detection label value.
The label
'007'is valid but unconventional for slop detection. Typically, labels such as "slop", "spam", or "low-quality" are used. Please confirm this is intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coderabbit.yml around lines 21 - 23, The slop_detection label currently uses the unconventional value '007'; verify whether this was intentional and if not change the slop_detection.label value to a more conventional identifier (e.g., "slop", "spam", or "low-quality") or document why '007' is required; update the slop_detection block (symbol: slop_detection.label) accordingly and ensure any downstream code or rules that reference this label are adjusted to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/continuous-integration.yml:
- Line 90: Ensure the workflow only upgrades to codecov/codecov-action@v6 after
verifying runner environments support Node.js 24: check your GitHub Enterprise /
self-hosted runner images for Node 24 or add a conditional that uses the current
v5 (codecov/codecov-action@v5) when Node 24 is unavailable; keep all existing
input parameters passed to the action (the same outputs/with keys) to maintain
compatibility; and for security/stability consider pinning the action to a
specific commit SHA (e.g., codecov/codecov-action@<sha>) instead of the loose v6
tag.
In `@tests/lib/database/settings/structures/PermissionNodeManager.test.ts`:
- Line 154: The test mistakenly uses createGuildMember to create a GuildMember
instead of a Role, so replace the line that creates the wrong entity with
creation of a Role (use the existing helper used elsewhere, e.g., createRole or
createGuildRole, or construct a Role for tests) so the test exercises
PermissionNodeManager.reset behaviour for a Role; update the variable name if
needed and ensure the created Role is passed into the reset/assertion calls that
currently reference role.
- Around line 119-131: The test doesn't assert that reset(user) actually threw,
so add a guard: either assert caughtError is defined before casting (e.g.,
expect(caughtError).toBeDefined()) or replace the try/catch with Vitest's
expect(() => reset(user)).toThrow() pattern; when using toThrow, assert the
thrown is an instance of UserError and that its identifier equals
LanguageKeys.Commands.Management.PermissionNodesNodeNotExists and context.target
equals user. Target symbols: reset, UserError,
LanguageKeys.Commands.Management.PermissionNodesNodeNotExists.
---
Nitpick comments:
In @.coderabbit.yml:
- Around line 21-23: The slop_detection label currently uses the unconventional
value '007'; verify whether this was intentional and if not change the
slop_detection.label value to a more conventional identifier (e.g., "slop",
"spam", or "low-quality") or document why '007' is required; update the
slop_detection block (symbol: slop_detection.label) accordingly and ensure any
downstream code or rules that reference this label are adjusted to match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ab87d86b-09d2-42b2-a833-6ba8c6e2590c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.coderabbit.yml.github/workflows/autofix.yml.github/workflows/continuous-integration.yml.github/workflows/update-tlds.yml.vscode/settings.jsoncompose.dev.yamlpackage.jsonpnpm-workspace.yamlsrc/lib/database/settings/index.tssrc/lib/moderation/workers/WorkerHandler.tstests/lib/database/settings/structures/PermissionNodeManager.test.tstests/lib/moderation/workers/WorkerHandler.test.tstests/lib/moderation/workers/WorkerManager.test.tstests/lib/util/functions/emojis/getEmojiTextFormat.test.tstsdown.config.ts
✅ Files skipped from review due to trivial changes (6)
- .github/workflows/update-tlds.yml
- compose.dev.yaml
- .github/workflows/autofix.yml
- .vscode/settings.json
- pnpm-workspace.yaml
- src/lib/database/settings/index.ts
Standalone Node ESM script that verifies the audit_event chain without depending on a TypeScript build: - Walks chain via prevHash links (timestamp-independent) - Detects HASH MISMATCH, CHAIN FORK, CYCLE, ORPHAN - Reports all rows as orphaned when no root exists - Uses raw pg pool + inline canonicalize/hashEnvelope - Reads DATABASE_URL from src/.env or environment - Exits 1 on any violation, 0 on clean chain
…wolfstar into feat/audit-log-manager
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/audit-verify.mjs (1)
93-158: Consider memory usage for large audit tables.The script loads all
audit_eventrows into memory (line 85). For guilds with extensive audit histories, this could consume significant memory. If you anticipate tables with hundreds of thousands of rows, consider streaming the verification or processing in batches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ce138f7d-6d4c-43f2-811b-6b516330d21f
📒 Files selected for processing (2)
package.jsonscripts/audit-verify.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
Member
Author
|
@copilot apply changes based on this thread |
Agent-Logs-Url: https://github.com/wolfstar-project/wolfstar/sessions/166db01e-8382-45f7-9a41-45933e67f964 Co-authored-by: RedStar071 <76824516+RedStar071@users.noreply.github.com>
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/wolfstar-project/wolfstar/sessions/166db01e-8382-45f7-9a41-45933e67f964 Co-authored-by: RedStar071 <76824516+RedStar071@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: RedStar071 <76824516+RedStar071@users.noreply.github.com>
- Add channelsLogsCommand and channelsLogsSettings guild settings fields - Add guild.command.execute action to audit action allowlist - Add AuditLogManager.command() method and channel emit fan-out - Add buildCommandExecuteEmbed and buildSettingsChangeEmbed helpers - Wire fan-out to chat-input, context-menu, and message command success listeners - Add en-US locale strings for audit log embeds
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #188 +/- ##
==========================================
+ Coverage 78.08% 78.21% +0.12%
==========================================
Files 81 82 +1
Lines 2154 2217 +63
Branches 211 229 +18
==========================================
+ Hits 1682 1734 +52
- Misses 449 453 +4
- Partials 23 30 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/listeners/commands/chatInputCommandSuccessLogger.ts (1)
16-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAudit logging is effectively disabled when debug logging is off.
The new audit write path is inside a listener that is only enabled in debug mode (Line 29). In non-debug environments, command audit events from this listener will never be recorded.
✅ Suggested fix
public run({ interaction }: ChatInputCommandSuccessPayload) { const shard = `[${cyan('0')}]`; const commandName = cyan(`/${interaction.commandName}`); const author = this.author(interaction.user); const sentAt = interaction.guildId ? `${interaction.guild?.name ?? 'Unknown'}[${cyan(interaction.guildId)}]` : cyan('Direct Messages'); - this.container.logger.debug(`${shard} - ${commandName} ${author} ${sentAt}`); + if (this.container.logger.has(LogLevel.Debug)) { + this.container.logger.debug(`${shard} - ${commandName} ${author} ${sentAt}`); + } @@ public override onLoad() { - this.enabled = this.container.logger.has(LogLevel.Debug); + this.enabled = true; return super.onLoad(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/listeners/commands/chatInputCommandSuccessLogger.ts` around lines 16 - 31, The audit write is gated by the listener being enabled only in debug mode via onLoad() (this.enabled = this.container.logger.has(LogLevel.Debug)), so move the audit logging out of that debug-only gate: remove or change the onLoad override so the listener (or at least the readSettingsAuditLog(...).command(...) call inside the handler using readSettingsCached and readSettingsAuditLog) runs regardless of LogLevel.Debug; alternatively extract the audit write into a separate always-enabled function/listener that calls readSettingsCached(interaction.guildId) and readSettingsAuditLog(...).command(...) so audit events are recorded even when container.logger.has(LogLevel.Debug) is false. Ensure references: onLoad, this.enabled, container.logger.has(LogLevel.Debug), readSettingsCached, readSettingsAuditLog, and the .command(...) call are updated accordingly.src/listeners/commands/messageCommandSuccessLogger.ts (1)
16-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMessage-command audit logging is tied to debug-only listener enablement.
Because
onLoad()disables this listener when debug is off, the new audit write path won’t execute in non-debug deployments.✅ Suggested fix
public run({ message, command }: MessageCommandSuccessPayload) { @@ - this.container.logger.debug(`${shard} - ${commandName} ${author} ${sentAt}`); + if (this.container.logger.has(LogLevel.Debug)) { + this.container.logger.debug(`${shard} - ${commandName} ${author} ${sentAt}`); + } @@ public override onLoad() { - this.enabled = this.container.logger.has(LogLevel.Debug); + this.enabled = true; return super.onLoad(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/listeners/commands/messageCommandSuccessLogger.ts` around lines 16 - 31, The listener's onLoad() currently disables the listener when LogLevel.Debug is not present, preventing the message-command audit writes; update onLoad() in messageCommandSuccessLogger (or remove the override) so the listener remains enabled in non-debug deployments (e.g., set this.enabled = true or simply call and return super.onLoad() without changing enabled) so the readSettingsAuditLog(...) call can run regardless of logger debug level while still calling super.onLoad().src/listeners/commands/contextMenuCommandSuccessLogger.ts (1)
16-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis listener also drops audit writes outside debug mode.
run()now performs command audit logging, butonLoad()still disables the entire listener unless debug logging is enabled. That prevents audit records in normal runtime configs.✅ Suggested fix
public run({ interaction }: ContextMenuCommandSuccessPayload) { @@ - this.container.logger.debug(`${shard} - ${commandName} ${author} ${sentAt}`); + if (this.container.logger.has(LogLevel.Debug)) { + this.container.logger.debug(`${shard} - ${commandName} ${author} ${sentAt}`); + } @@ public override onLoad() { - this.enabled = this.container.logger.has(LogLevel.Debug); + this.enabled = true; return super.onLoad(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/listeners/commands/contextMenuCommandSuccessLogger.ts` around lines 16 - 31, The listener's onLoad currently gates the whole listener behind container.logger.has(LogLevel.Debug), causing run()'s audit writes (readSettingsAuditLog / command audit) to be skipped outside debug mode; change onLoad so the listener remains enabled (e.g. set this.enabled = true or remove the LogLevel.Debug gating) and, if you still need debug-only behavior for verbose logging, check container.logger.has(LogLevel.Debug) inside run() or a separate flag rather than disabling the listener itself; update the onLoad method accordingly so audit logging always runs while keeping any debug-only logging conditional.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.oxlintrc.json:
- Line 139: The ignore pattern "src/languages/**" in the ignorePatterns entry is
too broad and excludes TypeScript handler files; update the ignorePatterns to
only exclude translation JSONs (e.g. replace the "src/languages/**" entry with a
narrower pattern such as "src/languages/**/*.json") so that files like index.ts
and the various Handler-extending constants files are linted; ensure you modify
the ignorePatterns array entry (the string "src/languages/**") accordingly.
In `@src/lib/util/functions/auditLogEmbeds.ts`:
- Around line 34-35: The log embed always prefixes commandName with "/" even for
non-slash commands; update the field construction in auditLogEmbeds.ts (the
object using LanguageKeys.Events.Guilds.Logs.LogFieldCommand) to only prefix
with "/" when commandType indicates a slash command (e.g., commandType ===
'slash' or whatever enum/string you use for slash commands), otherwise render
commandName as-is (no leading slash); adjust the value expression so it
conditionally formats `\`/${commandName}\`` vs ``\`${commandName}\`` depending
on commandType.
In `@tests/lib/database/settings/structures/AuditLogManager.test.ts`:
- Around line 257-263: Replace the loose "no emit" assertions that use
not.toHaveBeenCalledWith(...) with strict zero-call assertions so we assert that
emit was never called; specifically change the assertions referencing emitSpy
and Events.GuildMessageLog (the block around the test using emitSpy and
'channelsLogsCommand') to use expect(emitSpy).not.toHaveBeenCalled() and do the
same for the second occurrence noted (the other block around lines 271-277) so
the test verifies no emits happened at all.
---
Outside diff comments:
In `@src/listeners/commands/chatInputCommandSuccessLogger.ts`:
- Around line 16-31: The audit write is gated by the listener being enabled only
in debug mode via onLoad() (this.enabled =
this.container.logger.has(LogLevel.Debug)), so move the audit logging out of
that debug-only gate: remove or change the onLoad override so the listener (or
at least the readSettingsAuditLog(...).command(...) call inside the handler
using readSettingsCached and readSettingsAuditLog) runs regardless of
LogLevel.Debug; alternatively extract the audit write into a separate
always-enabled function/listener that calls
readSettingsCached(interaction.guildId) and
readSettingsAuditLog(...).command(...) so audit events are recorded even when
container.logger.has(LogLevel.Debug) is false. Ensure references: onLoad,
this.enabled, container.logger.has(LogLevel.Debug), readSettingsCached,
readSettingsAuditLog, and the .command(...) call are updated accordingly.
In `@src/listeners/commands/contextMenuCommandSuccessLogger.ts`:
- Around line 16-31: The listener's onLoad currently gates the whole listener
behind container.logger.has(LogLevel.Debug), causing run()'s audit writes
(readSettingsAuditLog / command audit) to be skipped outside debug mode; change
onLoad so the listener remains enabled (e.g. set this.enabled = true or remove
the LogLevel.Debug gating) and, if you still need debug-only behavior for
verbose logging, check container.logger.has(LogLevel.Debug) inside run() or a
separate flag rather than disabling the listener itself; update the onLoad
method accordingly so audit logging always runs while keeping any debug-only
logging conditional.
In `@src/listeners/commands/messageCommandSuccessLogger.ts`:
- Around line 16-31: The listener's onLoad() currently disables the listener
when LogLevel.Debug is not present, preventing the message-command audit writes;
update onLoad() in messageCommandSuccessLogger (or remove the override) so the
listener remains enabled in non-debug deployments (e.g., set this.enabled = true
or simply call and return super.onLoad() without changing enabled) so the
readSettingsAuditLog(...) call can run regardless of logger debug level while
still calling super.onLoad().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: d389a42a-f179-48be-99ae-9fca0cb2e4a1
📒 Files selected for processing (18)
.oxlintrc.jsonprisma/migrations/20260516000000_add_channels_logs_command_settings/migration.sqlprisma/schema.prismasrc/languages/en-US/events/guilds-logs.jsonsrc/lib/database/settings/auditActions.tssrc/lib/database/settings/constants.tssrc/lib/database/settings/structures/AuditLogManager.tssrc/lib/database/settings/types.tssrc/lib/i18n/languageKeys/keys/events/guilds/Logs.tssrc/lib/util/functions/auditLogEmbeds.tssrc/lib/util/functions/index.tssrc/listeners/commands/chatInputCommandSuccessLogger.tssrc/listeners/commands/contextMenuCommandSuccessLogger.tssrc/listeners/commands/messageCommandSuccessLogger.tstests/lib/database/settings/functions.test.tstests/lib/database/settings/structures/AuditLogManager.test.tstests/lib/database/settings/structures/PermissionNodeManager.test.tstests/lib/util/functions/auditLogEmbeds.test.ts
✅ Files skipped from review due to trivial changes (2)
- prisma/migrations/20260516000000_add_channels_logs_command_settings/migration.sql
- src/lib/util/functions/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/database/settings/auditActions.ts
- prisma/schema.prisma
Member
Author
|
@copilot Verify each finding against current code. Fix only still-valid issues, skip the Inline comments:
In
In
Outside diff comments:
In
In
|
Agent-Logs-Url: https://github.com/wolfstar-project/wolfstar/sessions/2c9c4193-c43c-4ce7-8969-607721ba3ba2 Co-authored-by: RedStar071 <76824516+RedStar071@users.noreply.github.com>
Contributor
... Applied in 2021cd8. I verified each finding against current code and all of them were still valid, so I fixed them with minimal changes: narrowed Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…context, and message commands
…nd execution logs
- AutoModerationCommand: chatInputRunEdit/Reset now use transactions - SetUpModerationCommand: inhibit() role assignment now audited - permissionNodes: add/remove/reset now use transactions with audit - create-mute: rolesMuted write now audited
- add _command-log-shared.ts with writeCommandLog and normalizeCommandError helpers - add 6 listener adapters for chat-input/message/context-menu success/error events - add authenticated paginated GET /guilds/:guild/command-logs route - add CommandLog model to prisma schema with migration - add tests for shared helper (14 passing) - fix chat-input slash prefix in formatChatInputMention fallback
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
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 an AuditLogManager to track dashboard settings changes, adapted from wolfstar.rocks PR #144 and following the same conventions as `PermissionNodeManager` and other existing managers.
Changes
Database
Core
Routes
Tests