Repository navigation
feat(bot): track guild join/leave history - #872
Conversation
Adds first-class persistence for which Discord servers the bot is in and when it was added to each, replacing the implicit (and inaccurate) reliance on `Guild.createdAt` for join time. Schema - guilds: add nullable `joinedAt` + `leftAt` for cheap current-state queries - new `guild_membership_events` table: immutable JOIN/LEAVE audit log keyed by Discord snowflake so history survives Guild row deletion - migration: 20260515000000_add_guild_membership_tracking Bot - new `guildMembershipService` with `recordGuildJoin`, `recordGuildLeave`, and `syncGuildsOnReady`; all writes go through a single transaction - `handleGuildCreate` event handler upserts the Guild and writes a JOIN event (using Discord-provided `guild.joinedTimestamp`) - `handleGuildDelete` now stamps `leftAt` and writes a LEAVE event before the existing cache cleanup - on ClientReady, idempotent backfill for any cached guild missing `joinedAt` (recovers history for guilds joined before this feature shipped or during downtime) Tests - guildMembershipService.spec: covers JOIN/LEAVE writes, fallback when `joinedTimestamp` is null, error swallowing, and on-ready idempotency - eventHandler.spec: stubs out the new service so existing tests stay green Foundation for upcoming PR B (bot observability) which will expose a `lucky_bot_guilds_total` gauge backed by this same data.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Failed to generate code suggestions for PR |
📝 WalkthroughWalkthroughThis PR introduces guild membership tracking to the bot by adding database tables for audit events, a service layer to record join/leave operations, and integration into bot event handlers with full test coverage. ChangesGuild Membership Tracking
Sequence DiagramsequenceDiagram
participant Bot as Bot Client
participant Handler as eventHandler
participant Service as guildMembershipService
participant Prisma
participant DB as Database
Bot->>Handler: clientReady event
Handler->>Service: syncGuildsOnReady(client)
loop for each cached guild
Service->>Prisma: findUnique Guild by discordId
Prisma->>DB: query Guild.joinedAt
alt joinedAt missing
Service->>Prisma: transaction()
Prisma->>DB: upsert Guild (set joinedAt)
end
end
Bot->>Handler: guildCreate event
Handler->>Service: recordGuildJoin(guild)
Service->>Prisma: transaction()
Prisma->>DB: upsert Guild (joinedAt, leftAt=null)
Prisma->>DB: create GuildMembershipEvent (JOIN)
DB-->>Prisma: confirmed
Prisma-->>Service: done
Bot->>Handler: guildDelete event
Handler->>Service: recordGuildLeave(guildId, guildName)
Service->>Prisma: transaction()
Prisma->>DB: updateMany Guild (set leftAt)
Prisma->>DB: create GuildMembershipEvent (LEAVE)
DB-->>Prisma: confirmed
Prisma-->>Service: done
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bot/src/handlers/eventHandler.spec.ts`:
- Around line 97-103: Add explicit assertions in the eventHandler.spec tests to
verify the mocked guild membership service functions are invoked when the
corresponding events fire: assert recordGuildJoinMock is called on the
GuildCreate test, recordGuildLeaveMock is called on the GuildDelete test, and
syncGuildsOnReadyMock is called on the clientReady test (also add similar
assertions in the other test blocks around lines 385-437 and 474-503). Locate
the existing jest.mock wiring that defines
recordGuildJoin/recordGuildLeave/syncGuildsOnReady and then add expect(...)
assertions referencing recordGuildJoinMock, recordGuildLeaveMock, and
syncGuildsOnReadyMock to confirm they were called with the expected args after
each simulated event.
In `@packages/bot/src/handlers/eventHandler.ts`:
- Around line 38-43: Replace the direct errorLog(...) calls in the three
fire-and-forget .catch(...) blocks (the one attached to syncGuildsOnReady(...)
and the two other similar catch blocks) with the logAndSwallow utility: import {
logAndSwallow } from '`@lucky/shared/utils/error`' and in each .catch pass the
caught error and a descriptive string (e.g., 'guildMembershipService: on-ready
sync failed') to logAndSwallow(error, '...') so failures are logged and
swallowed per guideline; update the three catch blocks accordingly and remove
the old errorLog usage there.
In `@packages/bot/src/services/guildMembershipService.ts`:
- Around line 15-40: The Prisma write queries currently return full records
unnecessarily; update each write call (e.g., prisma.guild.upsert,
prisma.guildMembershipEvent.create and the other write calls referenced around
the 67-74 and 105-122 ranges) to include a minimal select clause such as select:
{ id: true } so the database only returns required fields (or no payload if not
needed); locate the upsert and create invocations in guildMembershipService.ts
and add the appropriate select objects to each Prisma call.
- Around line 42-48: Replace the direct errorLog() calls in
guildMembershipService with the logAndSwallow utility to standardize error
handling: in the catch blocks for the JOIN handler (e.g., recordGuildJoin or the
function that records guild joins), the LEAVE handler (recordGuildLeave), and
the ready sync handler (syncGuildsOnReady), call logAndSwallow(error,
'guildMembershipService: failed to record JOIN' or the appropriate message for
LEAVE/SYNC) and pass the same context object (e.g., { guildId: guild.id, name:
guild.name } or relevant ids) instead of errorLog({...}); keep the original
message and context keys but move them into the logAndSwallow parameters.
🪄 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.yaml
Review profile: CHILL
Plan: Pro
Run ID: bcc77e01-ce70-4da7-ab71-7e513b7482ba
📒 Files selected for processing (6)
packages/bot/src/handlers/eventHandler.spec.tspackages/bot/src/handlers/eventHandler.tspackages/bot/src/services/guildMembershipService.spec.tspackages/bot/src/services/guildMembershipService.tsprisma/migrations/20260515000000_add_guild_membership_tracking/migration.sqlprisma/schema.prisma
| jest.mock('../services/guildMembershipService', () => ({ | ||
| recordGuildJoin: async (...args: unknown[]) => recordGuildJoinMock(...args), | ||
| recordGuildLeave: async (...args: unknown[]) => | ||
| recordGuildLeaveMock(...args), | ||
| syncGuildsOnReady: async (...args: unknown[]) => | ||
| syncGuildsOnReadyMock(...args), | ||
| })) |
There was a problem hiding this comment.
Add direct assertions for guild membership service integrations.
The new mocks are wired, but there’s no explicit assertion that recordGuildJoin, recordGuildLeave, and syncGuildsOnReady are called on GuildCreate/GuildDelete/clientReady. Please add those checks to lock in the new behavior.
Also applies to: 385-437, 474-503
🤖 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 `@packages/bot/src/handlers/eventHandler.spec.ts` around lines 97 - 103, Add
explicit assertions in the eventHandler.spec tests to verify the mocked guild
membership service functions are invoked when the corresponding events fire:
assert recordGuildJoinMock is called on the GuildCreate test,
recordGuildLeaveMock is called on the GuildDelete test, and
syncGuildsOnReadyMock is called on the clientReady test (also add similar
assertions in the other test blocks around lines 385-437 and 474-503). Locate
the existing jest.mock wiring that defines
recordGuildJoin/recordGuildLeave/syncGuildsOnReady and then add expect(...)
assertions referencing recordGuildJoinMock, recordGuildLeaveMock, and
syncGuildsOnReadyMock to confirm they were called with the expected args after
each simulated event.
| syncGuildsOnReady(client).catch((error) => { | ||
| errorLog({ | ||
| message: 'guildMembershipService: on-ready sync failed', | ||
| error, | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant sections
wc -l packages/bot/src/handlers/eventHandler.tsRepository: LucasSantana-Dev/Lucky
Length of output: 112
🏁 Script executed:
# Read the file with line numbers to see the context around the mentioned lines
head -80 packages/bot/src/handlers/eventHandler.ts | cat -nRepository: LucasSantana-Dev/Lucky
Length of output: 3359
🏁 Script executed:
# Also check the specific line ranges mentioned (61-66, 229-234)
sed -n '55,75p' packages/bot/src/handlers/eventHandler.ts | cat -nRepository: LucasSantana-Dev/Lucky
Length of output: 812
🏁 Script executed:
# Search for the logAndSwallow and logAndRethrow utilities
fd -t f '*.ts' -x grep -l 'logAndSwallow\|logAndRethrow' {} \;Repository: LucasSantana-Dev/Lucky
Length of output: 350
🏁 Script executed:
# Look for the errorLog utility import/definition
rg 'errorLog|logAndSwallow|logAndRethrow' packages/bot/src/handlers/eventHandler.ts -B 2 -A 2Repository: LucasSantana-Dev/Lucky
Length of output: 2224
🏁 Script executed:
# Search for logAndSwallow and logAndRethrow definitions
rg 'export.*logAndSwallow|export.*logAndRethrow|function logAndSwallow|function logAndRethrow' packages/Repository: LucasSantana-Dev/Lucky
Length of output: 379
🏁 Script executed:
# Check line 229-234
sed -n '225,240p' packages/bot/src/handlers/eventHandler.ts | cat -nRepository: LucasSantana-Dev/Lucky
Length of output: 781
🏁 Script executed:
# Search if these utilities are used anywhere in the codebase
rg 'logAndSwallow|logAndRethrow' --type ts packages/Repository: LucasSantana-Dev/Lucky
Length of output: 4721
🏁 Script executed:
# Read the logAndRethrow and logAndSwallow implementation
cat -n packages/shared/src/utils/error/logAndRethrow.tsRepository: LucasSantana-Dev/Lucky
Length of output: 1720
Use logAndSwallow() utility in the three error handler catch blocks instead of errorLog() directly.
Lines 38-43, 61-66, and 229-234 use direct errorLog() calls in .catch() blocks. Per coding guidelines, use the logAndSwallow() utility for operations where failure is acceptable and errors are swallowed. These are fire-and-forget operations, so logAndSwallow() is the appropriate choice.
Import from @lucky/shared/utils/error and wrap each catch block accordingly:
.catch((error) => {
logAndSwallow(error, 'guildMembershipService: on-ready sync failed')
})
🤖 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 `@packages/bot/src/handlers/eventHandler.ts` around lines 38 - 43, Replace the
direct errorLog(...) calls in the three fire-and-forget .catch(...) blocks (the
one attached to syncGuildsOnReady(...) and the two other similar catch blocks)
with the logAndSwallow utility: import { logAndSwallow } from
'`@lucky/shared/utils/error`' and in each .catch pass the caught error and a
descriptive string (e.g., 'guildMembershipService: on-ready sync failed') to
logAndSwallow(error, '...') so failures are logged and swallowed per guideline;
update the three catch blocks accordingly and remove the old errorLog usage
there.
| prisma.guild.upsert({ | ||
| where: { discordId: guild.id }, | ||
| create: { | ||
| discordId: guild.id, | ||
| name: guild.name, | ||
| icon: guild.icon ?? null, | ||
| ownerId: guild.ownerId, | ||
| joinedAt, | ||
| leftAt: null, | ||
| }, | ||
| update: { | ||
| name: guild.name, | ||
| icon: guild.icon ?? null, | ||
| ownerId: guild.ownerId, | ||
| joinedAt, | ||
| leftAt: null, | ||
| }, | ||
| }), | ||
| prisma.guildMembershipEvent.create({ | ||
| data: { | ||
| guildDiscordId: guild.id, | ||
| guildName: guild.name, | ||
| kind: 'JOIN', | ||
| occurredAt: joinedAt, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/bot/src/services/guildMembershipService.ts | head -150Repository: LucasSantana-Dev/Lucky
Length of output: 5443
Add explicit select on Prisma write queries to avoid returning unused payloads.
The upsert and create calls return full records by default, but none of the returned fields are used. Add minimal select (e.g., { id: true }) to keep data transfer bounded.
This applies to lines 15-40, 67-74, and 105-122 where Prisma queries lack explicit select or include clauses.
🤖 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 `@packages/bot/src/services/guildMembershipService.ts` around lines 15 - 40,
The Prisma write queries currently return full records unnecessarily; update
each write call (e.g., prisma.guild.upsert, prisma.guildMembershipEvent.create
and the other write calls referenced around the 67-74 and 105-122 ranges) to
include a minimal select clause such as select: { id: true } so the database
only returns required fields (or no payload if not needed); locate the upsert
and create invocations in guildMembershipService.ts and add the appropriate
select objects to each Prisma call.
| } catch (error) { | ||
| errorLog({ | ||
| message: 'guildMembershipService: failed to record JOIN', | ||
| data: { guildId: guild.id, name: guild.name }, | ||
| error, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and view the relevant sections
fd "guildMembershipService.ts" -t fRepository: LucasSantana-Dev/Lucky
Length of output: 118
🏁 Script executed:
# If the file exists, read the relevant lines and full context
if [ -f "packages/bot/src/services/guildMembershipService.ts" ]; then
echo "=== Full file content ===" && wc -l packages/bot/src/services/guildMembershipService.ts && echo "" &&
cat -n packages/bot/src/services/guildMembershipService.ts
fiRepository: LucasSantana-Dev/Lucky
Length of output: 5526
🏁 Script executed:
# Search for logAndSwallow and logAndRethrow utilities
rg "export.*logAnd(Swallow|Rethrow)" -t tsRepository: LucasSantana-Dev/Lucky
Length of output: 379
🏁 Script executed:
# Also check if errorLog is defined and how it's used
rg "export.*errorLog|function errorLog|const errorLog" -t tsRepository: LucasSantana-Dev/Lucky
Length of output: 4208
🏁 Script executed:
# Check what's exported from `@lucky/shared/utils`
rg "export.*from.*logAndRethrow|export.*logAndSwallow" packages/shared/src/utils -A 1Repository: LucasSantana-Dev/Lucky
Length of output: 421
🏁 Script executed:
# View the implementation of logAndSwallow
cat packages/shared/src/utils/error/logAndRethrow.tsRepository: LucasSantana-Dev/Lucky
Length of output: 1391
🏁 Script executed:
# Verify the export chain from the main utils index
cat packages/shared/src/utils/index.ts | grep -A 20 "error"Repository: LucasSantana-Dev/Lucky
Length of output: 386
🏁 Script executed:
# Check for existing usage of logAndSwallow in the codebase to confirm adoption pattern
rg "logAndSwallow\(" packages/bot packages/backend packages/shared -t ts -cRepository: LucasSantana-Dev/Lucky
Length of output: 407
Replace direct errorLog() calls with the logAndSwallow() utility.
All three error handlers (lines 42–48, 76–82, 124–130) currently bypass the standard error utilities. Per the coding guidelines, use logAndSwallow() to ensure consistent error context and propagation semantics:
} catch (error) {
logAndSwallow(error, 'guildMembershipService: failed to record JOIN', {
guildId: guild.id,
name: guild.name,
})
}Apply the same pattern to the recordGuildLeave and syncGuildsOnReady handlers.
🤖 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 `@packages/bot/src/services/guildMembershipService.ts` around lines 42 - 48,
Replace the direct errorLog() calls in guildMembershipService with the
logAndSwallow utility to standardize error handling: in the catch blocks for the
JOIN handler (e.g., recordGuildJoin or the function that records guild joins),
the LEAVE handler (recordGuildLeave), and the ready sync handler
(syncGuildsOnReady), call logAndSwallow(error, 'guildMembershipService: failed
to record JOIN' or the appropriate message for LEAVE/SYNC) and pass the same
context object (e.g., { guildId: guild.id, name: guild.name } or relevant ids)
instead of errorLog({...}); keep the original message and context keys but move
them into the logAndSwallow parameters.
|
## Summary PR B of the observability rollout. Exposes a Prometheus scrape target on the bot so the homelab observability stack (Prom + Grafana + Loki + OTel Collector, already deployed) can pull operational metrics directly. Builds on the schema added in #872. ## What ### Endpoint (new) - Tiny HTTP server on `METRICS_PORT` (default \`9091\`), bound to \`0.0.0.0\` so the in-cluster Prometheus can reach it - \`GET /metrics\` → Prometheus text exposition - \`GET /healthz\` → 200 OK while Discord client is ready, 503 otherwise (good for k8s/Docker healthchecks) - 405 on non-GET, 404 on unknown routes - \`METRICS_DISABLED=true\` skips startup (local CLI runs, tests) ### Metrics - **\`lucky_bot_guilds_total{service,state}\`** — gauge backed by the \`guilds\` table from #872 - \`state="active"\` — guilds where \`leftAt IS NULL\` (bot is currently in) - \`state="left"\` — guilds where \`leftAt IS NOT NULL\` (bot was removed from) - Computed lazily on each scrape via prom-client's \`collect()\` callback so there's no drift between in-process state and the database - **\`prom-client\` defaults**: \`process_cpu_user_seconds_total\`, \`nodejs_eventloop_lag_seconds\`, heap, GC, etc. ### Lifecycle - Server starts after the Discord client logs in (in \`initializer.ts\`, immediately after \`startClient\`) - Stopped during graceful shutdown; \`closeAllConnections()\` prevents keep-alive scrape sockets from blocking shutdown ## Test plan - [x] \`prometheus.spec\` (4 cases): gauge collect() reads Prisma counts; swallows DB errors; default Node metrics registered; correct content-type - [x] \`metricsServer.spec\` (7 cases): /metrics serves text exposition; /healthz reflects client.isReady(); 405/404 routing; METRICS_DISABLED no-op; idempotent start - [x] Bot \`tsc --noEmit\` clean - [x] All 50 affected suites green (eventHandler, guildMembership, initializer, prometheus, metricsServer) - [ ] Manual scrape on staging once deployed: \`curl bot:9091/metrics | grep lucky_bot_guilds_total\` ## Deployment notes - New env var: \`METRICS_PORT\` (default 9091) — set in docker-compose / k8s if you want a different port - Open port \`9091\` between the Prom scraper and the bot container (network-scope only; do NOT expose publicly) - Add scrape config to Prometheus: \`job_name: lucky-bot\`, \`static_configs.targets: [bot:9091]\` ## Out of scope - **PR C**: backend observability (Sentry + OTel HTTP + Prom) - **PR D**: frontend \`@sentry/react\` + web-vitals - **PR E**: Grafana dashboards + alert rules using \`lucky_bot_guilds_total\` and the rest of these metrics
## Summary Cut v2.13.0 of Lucky. Bumps root + 4 workspaces from `2.11.0` → `2.13.0` (skipping the archived `2.12.0`) and promotes the CHANGELOG `[Unreleased]` block to `[2.13.0] - 2026-05-21`. ## Headline changes since v2.11.0 **Added** - Guild Automation Module Executor seam + AutoMessages pilot (#901) - Sentry React SDK + Router v7 tracing/replay on frontend (#876) - Prometheus `/metrics` on backend (#875) + bot (#873) - Guild join/leave history tracking (#872) - Trivy image-scan on docker-publish, Phase A audit-only (#883) - Self-hosted developer-tooling register on landing page (#868) **Changed** - Backend migrated to Zod 4 API (#919) — unblocked the CVE patch + ended the lockfile fragility loop - 3 bot circular-deps clusters broken (#885, #886, #888) **Fixed** - brace-expansion DoS + ws uninit-memory CVEs patched (#921) - nginx-alpine CVEs (#881) - CI postinstall rate limit + madge actionlint (#878, #905) Full list in CHANGELOG.md. ## Next steps (after this PR merges) 1. Open `release/v2.13.0 → main` PR with merge-commit method 2. Tag `v2.13.0` on the merge commit 3. Cut next `release` (homelab-style bare branch) — Lucky's bare-release migration is still pending the user removing protection on `release/v2.11.0`
## Release v2.13.0 Promotes \`release/v2.13.0\` to \`main\` for the v2.13.0 cut. **$AHEAD commits across all merged PRs since v2.11.0 ship.** (Skipping v2.12.0 — the branch existed but its work was rolled forward into v2.13.0 alongside this session's Zod migration + CVE patches + standards adoption.) ## Headline changes **Added** — Guild Automation Module Executor pilot (#901), Sentry frontend (#876), Prometheus metrics on bot+backend (#873, #875), guild membership history (#872), Trivy image-scan Phase A (#883), landing redesign (#868). **Changed** — Backend migrated to Zod 4 API (#919), 3 bot circular-deps clusters broken (#885/#886/#888). **Fixed** — brace-expansion + ws moderate CVEs (#921), nginx-alpine CVEs (#881), CI postinstall rate limit (#878), madge actionlint (#905). **Internal** — shared coverageThreshold gate (#909/#914), Feature-removal sweep checklist + dangerfile guard (#908/#913), monitoring network, AI-doc policy, 4 new ADRs. Full list in [CHANGELOG.md](./CHANGELOG.md). ## Merge method This PR should land via **merge commit** (NOT squash) to preserve the individual PR SHAs in main's history. After merge: 1. Tag \`v2.13.0\` on the merge commit 2. Create GitHub release with notes from CHANGELOG.md 3. Fast-forward \`release/v2.13.0\` to match the new main HEAD ## Test plan - [ ] All 30 checks green except infra (snyk plan cap) - [ ] Verify \`gh pr view 922 --json mergeCommit\` shows the chore-bump commit on release tip - [ ] After merge: confirm \`origin/main\` contains the full $AHEAD commits



Summary
Adds first-class persistence for which Discord servers the bot is in and when it was added to each. Today we infer join time from
Guild.createdAt, which is just the row-insert timestamp — not the actual Discord join. This PR fixes that and adds an immutable audit trail for "servers over time" analytics.This is PR A of the observability rollout (see /observe routing decision). PRs B–E (bot/backend/frontend instrumentation + monitoring practice) will build on this foundation; the next planned step is exposing a `lucky_bot_guilds_total` Prometheus gauge sourced from this table.
Schema changes
Bot changes
Test plan
Out of scope (follow-up PRs)
Summary by CodeRabbit
New Features
Tests