Repository navigation
chore: remove the download feature for top.gg compliance - #1956
Conversation
reminders.guildId had no FK to guilds since the table was created (20260703050000_reminders never added it despite schema.prisma declaring the relation) - guild deletes did not cascade to reminders. afk_statuses was likewise missing its declared guildId index. Found via prisma migrate diff while generating an unrelated migration. Closes #1955
Top.gg's bot guidelines prohibit distributing downloads of copyrighted material without licensing. /download used yt-dlp to rip audio/video from YouTube, Spotify, and SoundCloud with no rights check - exactly that pattern, and a likely rejection cause for the pending listing review. Removing it entirely (not gating it) before resubmission. Drops the download command tree, the yt-dlp wrapper and download-path utils, the Download prisma model + guild_settings download columns, the download feature toggle (bot/shared/frontend), and the now-dead youtube-dl-exec dependency. See decisions/2026-08-09-remove-download-feature-topgg-compliance.md
📝 WalkthroughWalkthroughThe pull request removes Lucky’s download subsystem across the bot, shared services, frontend toggles, database schema, configuration, tests, and documentation. It also adds a separate schema-drift migration and records the decision in an ADR. ChangesDownload feature removal
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
github-actions
Bot
added
dependencies
|
Failed to generate code suggestions for PR |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@prisma/migrations/20260810014435_fix_schema_drift/migration.sql`:
- Around line 8-15: Make the migration operations in the SQL migration safely
re-runnable: conditionally add the reminders_guildId_fkey constraint only when
the equivalent foreign key is absent, conditionally create
afk_statuses_guildId_idx only when that index is absent, and conditionally
rename GuildForumThread_guildId_threadId_key only when the source exists and the
target does not. Preserve the intended foreign-key definition and index names.
- Around line 8-12: Update the migration to check for orphaned reminders.guildId
rows before adding the foreign key, then add reminders_guildId_fkey as NOT VALID
and validate it in a later step. Replace the regular afk_statuses_guildId_idx
creation with CREATE INDEX CONCURRENTLY, and configure or split the migration so
this non-transactional index operation runs outside Prisma’s migration
transaction.
🪄 Autofix
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 Plus
Run ID: 8ae34de3-6ab0-4d39-8eb1-de851afbed9c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (55)
README.mddecisions/2026-08-09-remove-download-feature-topgg-compliance.mddocs/ARCHITECTURE.mdpackages/backend/tests/integration/routes/toggles.test.tspackages/bot/package.jsonpackages/bot/src/config/constants.tspackages/bot/src/functions/download/commands/download.tspackages/bot/src/functions/download/commands/download/command.spec.tspackages/bot/src/functions/download/commands/download/command.tspackages/bot/src/functions/download/commands/download/index.tspackages/bot/src/functions/download/commands/download/processor.spec.tspackages/bot/src/functions/download/commands/download/processor.tspackages/bot/src/functions/download/commands/download/service.spec.tspackages/bot/src/functions/download/commands/download/service.tspackages/bot/src/functions/download/commands/download/types.tspackages/bot/src/functions/download/commands/download/validator.spec.tspackages/bot/src/functions/download/commands/download/validator.tspackages/bot/src/functions/download/commands/index.tspackages/bot/src/functions/download/utils/deleteContent.spec.tspackages/bot/src/functions/download/utils/deleteContent.tspackages/bot/src/functions/download/utils/downloadUtils.tspackages/bot/src/functions/download/utils/downloadVideo/index.tspackages/bot/src/functions/download/utils/downloadVideo/service.tspackages/bot/src/functions/download/utils/downloadVideo/types.tspackages/bot/src/functions/download/utils/ytDlpUtils.tspackages/bot/src/functions/download/utils/ytDlpUtils/downloader/index.tspackages/bot/src/functions/download/utils/ytDlpUtils/downloader/pathManager.tspackages/bot/src/functions/download/utils/ytDlpUtils/downloader/service.spec.tspackages/bot/src/functions/download/utils/ytDlpUtils/downloader/service.tspackages/bot/src/functions/download/utils/ytDlpUtils/downloader/types.tspackages/bot/src/functions/download/utils/ytDlpUtils/index.tspackages/bot/src/functions/download/utils/ytDlpUtils/pathManager.spec.tspackages/bot/src/functions/download/utils/ytDlpUtils/pathManager.tspackages/bot/src/functions/download/utils/ytDlpUtils/types.tspackages/bot/src/functions/management/commands/helpers/serversetupCriativaria.tspackages/bot/src/register.tspackages/bot/src/utils/command/commandCategory.tspackages/bot/src/utils/download/downloadHelpers.spec.tspackages/bot/src/utils/download/downloadHelpers.tspackages/frontend/src/hooks/useFeatures.test.tspackages/frontend/src/stores/featuresStore.tspackages/frontend/src/types/feature.tspackages/frontend/tests/e2e/features-page.spec.tspackages/frontend/tests/e2e/fixtures/test-data.tspackages/shared/src/__tests__/services/GuildSettingsService.test.tspackages/shared/src/config/config.tspackages/shared/src/config/constants.tspackages/shared/src/config/featureToggles.tspackages/shared/src/services/FeatureToggleService.spec.tspackages/shared/src/services/GuildSettingsService.spec.tspackages/shared/src/services/GuildSettingsService.tspackages/shared/src/types/featureToggle.tsprisma/migrations/20260810014435_fix_schema_drift/migration.sqlprisma/migrations/20260810014436_remove_download_feature/migration.sqlprisma/schema.prisma
💤 Files with no reviewable changes (43)
- packages/bot/package.json
- packages/bot/src/functions/download/utils/deleteContent.ts
- packages/bot/src/functions/download/utils/downloadVideo/types.ts
- packages/bot/src/functions/management/commands/helpers/serversetupCriativaria.ts
- packages/shared/src/types/featureToggle.ts
- packages/bot/src/config/constants.ts
- packages/bot/src/functions/download/commands/index.ts
- packages/shared/src/services/GuildSettingsService.spec.ts
- README.md
- packages/bot/src/functions/download/commands/download/types.ts
- packages/frontend/src/stores/featuresStore.ts
- packages/bot/src/functions/download/utils/ytDlpUtils.ts
- packages/bot/src/utils/download/downloadHelpers.ts
- packages/bot/src/functions/download/commands/download/index.ts
- packages/frontend/src/types/feature.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/pathManager.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/downloader/index.ts
- packages/bot/src/functions/download/commands/download/command.ts
- packages/bot/src/register.ts
- packages/bot/src/functions/download/utils/deleteContent.spec.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/index.ts
- packages/bot/src/utils/download/downloadHelpers.spec.ts
- packages/bot/src/functions/download/commands/download/command.spec.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/downloader/pathManager.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/downloader/service.spec.ts
- packages/frontend/tests/e2e/features-page.spec.ts
- packages/bot/src/functions/download/commands/download/processor.ts
- packages/bot/src/functions/download/utils/downloadUtils.ts
- packages/bot/src/functions/download/commands/download/service.ts
- packages/bot/src/functions/download/commands/download/validator.ts
- packages/bot/src/functions/download/commands/download/processor.spec.ts
- packages/shared/src/tests/services/GuildSettingsService.test.ts
- packages/bot/src/functions/download/utils/downloadVideo/service.ts
- packages/shared/src/services/GuildSettingsService.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/downloader/types.ts
- packages/bot/src/functions/download/utils/downloadVideo/index.ts
- packages/bot/src/functions/download/commands/download/service.spec.ts
- packages/frontend/tests/e2e/fixtures/test-data.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/types.ts
- packages/bot/src/functions/download/commands/download/validator.spec.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/downloader/service.ts
- packages/bot/src/functions/download/commands/download.ts
- packages/bot/src/functions/download/utils/ytDlpUtils/pathManager.spec.ts
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the bot for Top.gg compliance. It deletes the download subsystem across packages (bot commands/utils/registration, shared service/types/config toggles and the Prisma Download model plus GuildSettings download columns, and the frontend feature-toggle option and e2e fixtures), removes the associated yt-dlp/download dependencies, and adds a DB migration dropping the downloads table and related columns. It also adds an ADR documenting the decision and updates README/architecture docs to drop download references, alongside some apparently unrelated changes to GuildSettingsService (repeat-count methods) and dependency updates (e.g. bullmq, tsx, discord.js).
No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 373 functions depend on the 302 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 15 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 373 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 363 function(s) in the blast radius were not formally verified this run
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 56 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Code review on #1956 caught it: adding reminders_guildId_fkey with no pre-flight cleanup fails outright at deploy time if any row references a since-deleted guild - and that's not hypothetical, one such orphan was hand-deleted in prod on 2026-07-28 for this exact table. Delete orphans before the constraint add.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the Lucky Discord bot for Top.gg compliance. It deletes the download command and its subsystem across the bot, shared, and frontend packages, drops the associated Prisma model and GuildSettings columns (via migration), removes related feature toggles, and cleans up dependencies like youtube-dl-exec/yt-dlp. It also adds an ADR documenting the decision and updates README and architecture docs to reflect the removal. The surface area spans documentation (README, ADR, architecture docs), the bot/shared/frontend GuildSettings and feature-toggle code, associated test fixtures and specs, and dependency lockfile changes.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 373 functions depend on the 302 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 15 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 373 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 363 function(s) in the blast radius were not formally verified this run
…-feature # Conflicts: # package-lock.json
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the bot for Top.gg compliance, along with an accompanying ADR (decisions/2026-08-09-remove-download-feature-topgg-compliance.md) documenting the rationale. The change spans the bot package (removing the download command subsystem, feature toggle, and related helpers), the shared package (dropping the Download Prisma model, GuildSettings.allowDownloads/downloadCooldown fields, and feature-toggle types), and the frontend (removing the download feature-toggle option in the features store and e2e fixtures). It also updates documentation (README command table, ARCHITECTURE.md package layouts) to drop download references and includes some incidental dependency lockfile updates. Surface area touched: bot command/helper files, shared services/config/types, frontend feature store, docs, and package/lockfile metadata.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 375 functions depend on the 303 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 375 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 365 function(s) in the blast radius were not formally verified this run
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the bot across all packages. Per the accompanying ADR, the change is motivated by Top.gg listing compliance and deletes the download command subsystem in the bot, the related Prisma Download model and GuildSettings columns in shared, the download feature-toggle option in the frontend, and adds DB migrations to drop the associated table and columns. The surface area spans documentation updates (README command table, ARCHITECTURE package layouts, and a new ADR file), shared service/type/config changes around guild settings and feature toggles, frontend feature-store and e2e fixture updates, and backend integration test adjustments. Bot package metadata (dependencies, scripts, version) and lockfile entries are also touched.
No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 375 functions depend on the 303 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 375 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 365 function(s) in the blast radius were not formally verified this run
…-feature # Conflicts: # package-lock.json
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature across the monorepo for Top.gg listing compliance. It deletes the download command subsystem, related Prisma models/columns, feature toggles, and frontend options in the bot, shared, and frontend packages, and updates documentation (README, ARCHITECTURE) plus adds an ADR explaining the rationale. It also includes a database migration to drop the downloads table and associated guild_settings columns, along with corresponding test/mock and dependency updates.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 375 functions depend on the 303 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 375 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 365 function(s) in the blast radius were not formally verified this run
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the bot for Top.gg compliance reasons. It deletes the download subsystem across packages (bot commands/processor/service, shared Prisma Download model and GuildSettings download columns, the download feature toggle in shared/frontend, and related e2e fixtures), adds a DB migration dropping the downloads table and two guild_settings columns, and updates documentation (README command list, architecture docs) to reflect the removal. It also adds an ADR (decisions/2026-08-09-...) explaining the rationale, and the diff includes some unrelated package-lock.json dependency bumps. Surface area spans the bot, shared, and frontend packages plus docs, DB migrations, and lockfile changes.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 375 functions depend on the 303 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 375 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 365 function(s) in the blast radius were not formally verified this run
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
This PR removes the /download feature from the Lucky Discord bot across the entire codebase for Top.gg listing compliance. It adds a new ADR documenting the rationale (the command used yt-dlp to download copyrighted media, which violates Top.gg guidelines), and deletes/edits the associated bot commands, shared services, feature toggles, Prisma model and columns, frontend toggle options, and related test fixtures, along with documentation updates in the README and ARCHITECTURE files. The surface area spans all packages (bot, shared, backend, frontend), database migrations, e2e/integration test fixtures, and dependency lockfile changes. The many listed symbol changes appear to reflect the wide-ranging removal of download references plus incidental updates to config, command registration, and package dependencies.
Worth a look
- Prisma migration drops guild_settings columns without data safety note —
decisions/2026-08-09-remove-download-feature-topgg-compliance.md· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 375 functions depend on the 303 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 375 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 365 function(s) in the blast radius were not formally verified this run
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the bot as part of a Top.gg compliance effort, documented in a new ADR (decisions/2026-08-09-remove-download-feature-topgg-compliance.md). The surface area spans multiple packages: the bot's download command/subsystem and its command-category/feature-toggle registration, shared services/types (GuildSettingsService, FeatureToggleService, feature toggle configs/types, and default settings), and the frontend feature-toggle store/types. It also updates the README command table and docs/ARCHITECTURE.md to reflect the new command categories, adds a backfill guild-settings script reference, and touches bot package.json dependencies plus the corresponding package-lock.json entries. Note: the diff was truncated, so this summary reflects the visible portions (documentation, ADR, architecture/README updates, and lockfile/dependency changes) and the changed-symbol list rather than the full download-removal implementation.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 383 functions depend on the 311 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 383 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 373 function(s) in the blast radius were not formally verified this run
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
This pull request removes the /download feature across the codebase for Top.gg listing compliance, per a new ADR (decisions/2026-08-09-remove-download-feature-topgg-compliance.md) that documents the rationale. Changes span the bot commands/helpers, shared services and types (GuildSettings, feature toggles), and frontend feature-toggle types/stores, along with documentation updates to the README and architecture docs to drop download references. It also includes package-lock/dependency adjustments (e.g., discord-player extractors, dotenv) and touches a guild-setup helper module and guild settings service specs. Surface area is broad but concentrated on removing the download subsystem and its references; reviewers should verify the removals are complete and consistent across packages and that the unrelated dependency/lockfile changes are intended.
Worth a look
- FeatureToggleName no longer accepts download —
packages/shared/src/types/featureToggle.ts· Escalate · medium · 2 independent checks- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 383 functions depend on the 311 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 383 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 373 function(s) in the blast radius were not formally verified this run
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
This pull request removes the /download command and its entire supporting subsystem across the bot, shared, and frontend packages, and documents the rationale (Top.gg compliance) in a new ADR. It also drops the related Download Prisma model, GuildSettings columns, and download feature toggle, with an accompanying database migration, and updates the README and architecture docs to reflect the removed feature and revised command categories. The lockfile changes reflect associated dependency adjustments.
Worth a look
- removeAutoplayCounter/toPrismaData may still reference dropped GuildSettings columns —
packages/shared/src/services/guildSettingsService.ts· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 383 functions depend on the 311 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 383 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 373 function(s) in the blast radius were not formally verified this run
…es (#2013) ## Problem Retries in `.github/workflows/ci.yml`'s `docker-build` job already set `use-cache: 'false'` to skip importing the gha layer cache after a `failed to calculate checksum of ref ...: not found` failure — but `cache-to` (mode=max export) stayed active regardless of `use-cache`. ## Evidence PR #1956 and #1952 (2026-08-14) both exhausted all 3 attempts (build + 2 retries) with the identical error, even though retry 1 and retry 2 both had `cache-from` disabled: ``` ERROR: failed to calculate checksum of ref ...: "/app/packages/backend/node_modules": not found ``` Cache import being off but the failure persisting identically means the export side alone can trigger the same checksum-resolution failure — `mode=max` has to solve a cache key for every intermediate stage, including the branching `installed-deps` checkpoint that two downstream stages (`source-copied` and `deps-production-base`) read from concurrently. ## Fix Gate `cache-to` by the same `use-cache` flag so retries run a genuinely cache-free build (no import, no export) instead of only skipping import. ## Test plan - [ ] This PR's own `docker-build` matrix job (backend/bot/frontend/nginx) passes, validating the composite action change works - [ ] Once merged, rebase #1956 and #1952 onto main and confirm their `Build — Docker images` required check goes green <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Disable `cache-to` on `docker-build` retries when `use-cache` is false to run a fully cache-free build and avoid the BuildKit “failed to calculate checksum of ref …: not found” error. Previously, retries disabled only `cache-from` while still exporting with `mode=max`, which could reproduce the same failure during export. - Change: In `.github/actions/docker-build-service/action.yml`, gate `cache-to` by `inputs.use-cache` (mirrors `cache-from`). First attempts unchanged; retries now skip both import and export. - Validate: CI `docker-build` matrix should pass and show no cache writes on retries. - Follow-up: Rebase #1956 and #1952 and confirm `Build — Docker images` passes. <sup>Written for commit 33bdce6. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/2013?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved build caching behavior during retries and cache-resolution failures. * Cache export is now disabled when caching is turned off, preventing unnecessary cache operations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the bot for Top.gg compliance, as documented in a new ADR (2026-08-09-remove-download-feature-topgg-compliance.md). The changes span multiple packages: deleting the download command subsystem and its feature toggle in the bot, removing related Prisma models/columns and service entries in shared, dropping the download feature-toggle option and e2e fixtures in the frontend, and adding a DB migration. Documentation (README, ARCHITECTURE) is updated to reflect the removed command and command-category structure, and there are incidental package-lock.json dependency version bumps.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 383 functions depend on the 311 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 383 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 373 function(s) in the blast radius were not formally verified this run
) ## Context Follow-up to #2013. That PR gated \`cache-to\` by \`use-cache\` on retries so they run fully cache-free — this definitively ruled out the gha cache as the cause (see #2002's latest comments): the identical \`failed to calculate checksum of ref ...: not found\` error reproduced 6/6 across PR #1956 and #1952 with cache completely disabled. Real cause is a local BuildKit race (filed as #2015) — not something to fix with another CI-config tweak, needs a Dockerfile restructure. ## Interim mitigation Since it's a race (not deterministic — some runs the same day succeeded), bump retries from 2 to 4 (5 attempts total) to raise the odds one attempt lands clean while #2015 is worked. ## Test plan - [ ] This PR's own docker-build check passes <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Increase CI docker-build retries from 2 to 4 (5 attempts total) to mitigate a non-deterministic BuildKit race that triggers "failed to calculate checksum of ref ...: not found". Previously we retried twice with cache disabled on retries; now we retry four times, still disabling cache on retries, trading longer worst-case time for higher pass rates until the Dockerfile fix in issue #2015. - Adds retry steps 2–4 and uses `continue-on-error: true` for the middle attempts to allow progressing to the next retry; the final attempt determines job failure. - Leaves the first attempt unchanged and keeps `use-cache: 'false'` on all retries. - No other workflow logic changes; service-specific `load` behavior remains the same. <sup>Written for commit 1c267f7. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/2016?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR removes the /download feature from the bot to comply with Top.gg's bot guidelines, which prohibit distributing downloads of copyrighted material. It deletes the download command subsystem, related Prisma models/columns (Download, allowDownloads, downloadCooldown), the download feature toggle across shared/frontend/bot, and associated dependencies, backed by a new ADR and DB migrations. The changes touch documentation (README, ARCHITECTURE), a new decision record, feature-toggle types and stores, guild settings services and their tests, command category utilities, frontend e2e fixtures, a backfill script, and package/lockfile dependency updates.
No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 383 functions depend on the 311 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 383 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 373 function(s) in the blast radius were not formally verified this run
Fixes the real root cause behind issue #2015 (and #2002): installed-deps is concurrently extended by a second live branch (source-copied, via FROM inheritance) for the rest of the build. deps-production-base's three COPY --from=installed-deps steps were reading that stage while it was still being written to elsewhere in the same build DAG, which hit a BuildKit race resolving the cross-stage ref -- "failed to calculate checksum of ref ...: not found" -- non-deterministically but at a very high rate on GH Actions runners (10/10 recent attempts across #1956 and #1952, even fully cache-free). Retries and cache tuning (#2013, #2016) didn't fix it because it isn't a caching issue. Running npm ci directly in deps-production-base (which already branches straight from node:\${NODE_VERSION}, never touching installed-deps or build) fully decouples this lineage: no more cross-stage read of a stage still being concurrently written to. Cost is one extra @discordjs/opus native compile, once per build (this stage is shared by both deps-production-bot and deps-production-backend), paid once instead of the 3-5 min routinely wasted on exhausted retries. Not locally build-verified end-to-end: native C compilation under QEMU cross-arch emulation (amd64 on this arm64 Mac, via colima) segfaults GCC independent of this change (cc: internal compiler error: Segmentation fault signal terminated program cc1) -- a known QEMU/cross-compile instability class, not present on real amd64 hardware. Relying on CI (real amd64 runners) to validate.
## Root cause (issue #2015) \`installed-deps\` (an empty checkpoint = \`FROM build AS installed-deps\`) is consumed by two different lineages within a single \`docker buildx build\` invocation for any \`production-*\` target: 1. \`source-copied\` (\`FROM installed-deps\`) → \`build-shared\` → \`build-backend\`/\`build-bot\`/\`build-frontend\` — inherits and keeps extending \`installed-deps\`'s filesystem with more COPY/RUN instructions. 2. \`deps-production-base\` did \`COPY --from=installed-deps /app/node_modules ...\` and \`COPY --from=installed-deps /app/packages/shared/node_modules ...\`; \`deps-production-bot\`/\`deps-production-backend\` each did one more \`COPY --from=installed-deps\` for their own package's node_modules. Both lineages read/extend \`installed-deps\` concurrently — BuildKit parallelizes independent branches of the stage DAG by default. The \`COPY --from\` reads in branch 2 race against branch 1 still actively extending the same stage, and intermittently fail: \`\`\` ERROR: failed to calculate checksum of ref ...: "/app/packages/backend/node_modules": not found \`\`\` Confirmed **not** a caching issue: reproduced 10/10 recent attempts across #1956 and #1952 with the gha cache fully disabled (#2013), and retries alone don't reliably dodge it even with 5 attempts (#2016). ## Fix \`deps-production-base\` already branches straight from \`node:${NODE_VERSION}\` (never touches \`installed-deps\` or \`build\`). Give it its own \`npm ci\` instead of copying node_modules out of \`installed-deps\` — this fully removes the cross-stage read of a stage still being concurrently written to. \`deps-production-bot\`/\`deps-production-backend\` no longer need any \`COPY --from=installed-deps\` at all (npm workspaces already installs every workspace's node_modules from the root \`npm ci\`). Cost: one extra \`@discordjs/opus\` native compile, paid once per build (this stage is shared by both bot and backend production targets) — versus the 3-5 min routinely wasted on exhausted retries recently. ## Verification Not locally build-verified end-to-end — native C compilation under QEMU cross-arch emulation (amd64 on an arm64 Mac via colima) segfaults GCC independent of this change (\`cc: internal compiler error: Segmentation fault signal terminated program cc1\`), a known QEMU/cross-compile instability class not present on real amd64 hardware. Relying on this PR's own CI (real amd64 runners, matching production) to validate. ## Test plan - [ ] This PR's own \`docker-build\` matrix (bot/backend/frontend/nginx) passes, ideally on the very first attempt (no retries needed) — confirms the race is actually gone, not just dodged - [ ] \`Verify native modules load — bot\` step still passes (opus still loads correctly from the independently-installed node_modules) - [ ] Once merged, rebase #1956 and #1952 and confirm their Docker build check goes green cleanly <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Decouples `deps-production-base` from `installed-deps` by running `npm ci` instead of copying `node_modules`. This removes a BuildKit race that intermittently failed production builds with checksum-not-found errors. - Replaces `COPY --from=installed-deps` with `npm ci` (with cache mount) in `deps-production-base`; removes the per-package `COPY --from=installed-deps` in `deps-production-bot` and `deps-production-backend`; keeps `npm prune --omit=dev`. - Fixes the root cause: `installed-deps` was read while another branch still extended it, triggering parallel BuildKit races (“failed to calculate checksum of ref …: not found”). - Impact: one extra `@discordjs/opus` native compile once per build; no runtime changes; workspace `npm ci` still installs all package `node_modules`. - Rollout: no migrations; CI should pass without retries. Rebase PRs previously failing on issue #2015 after merge. <sup>Written for commit 1b1a877. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/2017?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved production container dependency installation and caching. * Streamlined separate production builds for bot and backend services. * Reduced reliance on shared dependency copies between build stages. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This pull request removes the /download feature across the codebase for Top.gg compliance, touching the bot commands/processor/service, shared Prisma models and GuildSettings columns, feature toggles, frontend feature-store options, and their e2e fixtures. It adds an ADR (decisions/2026-08-09-remove-download-feature-topgg-compliance.md) documenting the rationale, updates the README and architecture docs to drop download references, and includes DB migrations to remove the downloads table and related columns. The diff also contains package-lock dependency version bumps and mentions a separate schema-drift migration tracked under issue #1955.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 383 functions depend on the 311 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 383 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 373 function(s) in the blast radius were not formally verified this run
Even fully isolated (no concurrent PR builds, no cache at all), the docker-build backend leg failed 5/5 attempts on both #1956 and #1952 with the same error the Dockerfile fix in #2017 partially addressed: failed to calculate checksum of ref ...: not found That fix (decoupling deps-production-base from installed-deps) validated clean once but didn't fully eliminate the race -- the same signature reappeared on a different COPY --from step (production-backend <- deps-production-backend), with the unrelated build stage's own npm ci still running concurrently in the log at the moment of failure. This is a BuildKit solver-level race under its default concurrent stage scheduling, confirmed independent of caching (#2013) and independent of cross-job/cross-PR concurrency (reproduces running fully alone). buildkitd-config-inline sets max-parallelism=1 on the ephemeral docker-container builder, forcing one stage operation at a time. Trades some build wall-clock for eliminating the race outright instead of retrying around it.
…#2018) ## Context Follow-up to #2017. That fix (decoupling `deps-production-base` from `installed-deps`) validated clean once (4/4 services, attempt 1, no retries) but didn't fully eliminate the race — rebuilding #1956 and #1952 afterward, both failed 5/5 attempts again with the identical error, now on a different `COPY --from` step (`production-backend` reading `deps-production-backend`), with the `build` stage's own unrelated `npm ci` still running concurrently in the log at the moment of failure. Ruled out as causes: gha cache (#2013 — fails identically fully cache-free), cross-job/cross-PR concurrency (fails identically running fully alone on an isolated rerun). This is a BuildKit solver-level race under its default concurrent stage scheduling — confirmed via Docker's own docs that `max-parallelism` is exactly the documented knob for "particularly useful for low-powered machines" style solver concurrency issues. ## Fix `buildkitd-config-inline` on the `docker/setup-buildx-action` step sets `max-parallelism = 1` (both `[worker.oci]` and `[worker.containerd]`, covering whichever worker the image uses), forcing BuildKit to execute one stage operation at a time instead of racing multiple branches concurrently. Trade-off: slower builds (no more parallel stage execution) in exchange for actually eliminating the race instead of retrying around it. Can tune back up (e.g. `max-parallelism = 2`) later if this proves too conservative once we have clean data. ## Test plan - [ ] This PR's own docker-build matrix passes on attempt 1 (no retries needed) — the real signal that the race is gone - [ ] Note build duration vs previous runs to gauge the serialization cost <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Serializes Docker BuildKit stage scheduling in CI and production publishing to eliminate intermittent "failed to calculate checksum of ref ...: not found" during COPY --from. Previously stages ran concurrently; now both workflows set max-parallelism=1, trading some build speed for reliability. **Details** - Configure `docker/setup-buildx-action` with `buildkitd-config-inline` setting `[worker.oci]` and `[worker.containerd]` `max-parallelism = 1` in `.github/actions/docker-build-service/action.yml` and `.github/workflows/docker-publish.yml`. - Update `.github/workflows/ci.yml` path filter to include `.github/actions/docker-build-service/` so docker-build runs when its composite action changes. - Clarify Dockerfile header on reproducing vs suppressing the race; no functional Dockerfile changes. <sup>Written for commit 5c23bea. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/2018?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Docker image build reliability by serializing build stages, helping prevent intermittent checksum-related failures. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This pull request removes the /download feature from the Lucky Discord bot across the entire monorepo for Top.gg compliance reasons, as documented in a new ADR. The changes span the bot package (removing download commands, category registration, and the download feature toggle), the shared package (removing the Download Prisma model, related GuildSettings columns, and feature-toggle types), and the frontend (removing the download toggle option and e2e fixtures), along with database migrations, README updates, and architecture documentation edits. The diff also includes package-lock.json dependency version bumps (e.g. AWS SDK, source-map) that appear incidental to the feature removal. Surface area touched includes command category helpers, config/environment parsing, feature toggle services and their tests, guild settings service and tests, and various server-setup helper symbols.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 383 functions depend on the 311 functions this change touches.
Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
replenishQueue()— 19 callers, 41 callees (high)requireDJRole()— 14 callers, 3 callees (high)runCriativariaSetup()— 3 callers, 13 callees (high)sendNowPlayingEmbed()— 3 callers, 12 callees (high)setupToggleRoutes()— 3 callers, 10 callees (medium)executePlayAtTop()— 3 callers, 9 callees (medium)ensureEnvironment()— 3 callers, 9 callees (medium)setupSupportRoutes()— 3 callers, 8 callees (medium)- …and 2 more
Verification — 383 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 373 function(s) in the blast radius were not formally verified this run
|
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
…ndeclared (#2040) ## Summary **Production-breaking regression, urgent.** After #2038's clean node_modules/lockfile regen (needed for the deepmerge-ts security fix) busted a stale Docker npm-install cache layer, YouTube extractor registration now throws on every bot boot: ``` Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'youtube-dl-exec' imported from /app/node_modules/discord-player-youtubei/dist/index.mjs YouTube extractor unavailable after retries — #play of YouTube URLs will fail until restart. Falling back to SoundCloud/Spotify only. ``` Discovered live on homelab after redeploying #2036 (the yt-dlp-cookies fix for #2034) — the cookies fix itself is correct, but it's moot because `createResilientStream` (the function it patches) never gets wired in at all when this registration fails, since it's passed as `{ createStream: createResilientStream }` at the `player.extractors.register(YoutubeExtractor, ...)` call site (`packages/bot/src/handlers/player/playerFactory.ts:166-169`). ## Root cause `discord-player-youtubei`'s compiled `dist/index.mjs` has a hard top-level `import youtubeDl from "youtube-dl-exec"` (confirmed: `grep -n youtube-dl-exec node_modules/discord-player-youtubei/dist/index.mjs`) that it never declares anywhere in its own `package.json` (dependencies/optionalDependencies/peerDependencies all empty) — an upstream packaging bug. `youtube-dl-exec` was a real dependency of ours until `a20a6c29` (5 days ago, "remove the download feature for top.gg compliance", #1956) correctly dropped it since our own `/download` command no longer used it. Nobody knew `discord-player-youtubei` also needed it internally. It kept working in production purely because Docker's npm-install layer (keyed by `package-lock.json` hash) was still cached from before that removal — until #2038's full lockfile regen busted that cache for the first time, surfacing the gap. ## Fix Re-declare `youtube-dl-exec` in `packages/bot/package.json` — purely as a transitive requirement for `discord-player-youtubei`, no download-feature code restored. `Dockerfile` already sets `YOUTUBE_DL_SKIP_DOWNLOAD=1` (see #1827/#874), so its postinstall binary-download stays skipped as before. ## Test plan - [x] `node -e "import('discord-player-youtubei').then(...)"` resolves cleanly, `YoutubeExtractor` export present - [x] `npx jest` (bot package) — 3139/3140 pass (1 pre-existing skip) - [x] `tsc --noEmit` clean - [x] `npm run audit:high` clean - [ ] Deploy + confirm `/play` of a YouTube URL works and the extractor-registration error is gone from logs <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Restores `youtube-dl-exec` in `packages/bot` to satisfy `discord-player-youtubei`’s undeclared import and fix the boot-time ERR_MODULE_NOT_FOUND that prevented YouTube extractor registration and playback. Documents `YOUTUBE_DL_SKIP_DOWNLOAD=1` for local installs to avoid postinstall downloads. - Declares `youtube-dl-exec` in `packages/bot/package.json` only for the extractor; no download feature is restored. - Behavior: YouTube extractor registers on boot; YouTube URLs play; no other code paths change. - Docs: README instructs `YOUTUBE_DL_SKIP_DOWNLOAD=1 npm install` for local dev; Docker and CI already skip postinstall. - Rollout: rebuild and redeploy the bot image. Developers must run `YOUTUBE_DL_SKIP_DOWNLOAD=1 npm install` locally. <sup>Written for commit 823c1e2. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/2040?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for retrieving media from YouTube and other supported platforms. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 participant
Summary
/downloadused yt-dlp to rip audio/video from YouTube/Spotify/SoundCloud with no rights check - a likely rejection cause for the pending listing review. Removed entirely (not gated) before resubmission.Downloadprisma model +guild_settingsdownload columns, thedownloadfeature toggle (bot/shared/frontend), and the now-deadyoutube-dl-execdep.remindershad no FK toguildssince the table was created, andafk_statuseswas missing its declared index. Tracked in data: reminders table shipped with no guildId FK; afk_statuses missing its index #1955.decisions/2026-08-09-remove-download-feature-topgg-compliance.mdTest plan
type:checkclean on all 4 packages (shared/bot/backend/frontend)prisma migrate resetreplayed both new migrations cleanly against local dev DB;prisma migrate statusconfirms no driftpackage-lock.jsonregenerated after droppingyoutube-dl-execSummary by cubic
Removes the
/downloadcommand and all download code for Top.gg compliance. Previously users could rip audio/video via yt‑dlp; now the command, related toggles, schema, and dependency are removed. Music playback is unchanged.packages/bot; removes thedownloadcategory from command constants inpackages/botand@lucky/shared; adds tests forcommandCategory.youtube-dl-execdependency and regenerates the lockfile.DOWNLOAD_VIDEO/DOWNLOAD_AUDIOfeature toggles across@lucky/shared, backend tests, and frontend; updates fixtures and e2e tests to useAUTOPLAY/LYRICS.downloadstable andguild_settings.allowDownloads/downloadCooldowncolumns; fixes drift by adding the missingreminders.guildIdFK with orphan cleanup and creating theafk_statuses.guildIdindex. All DDL is guarded per-table for safe, idempotent replays.docs/ARCHITECTURE.md, adds an ADR, and removes stale fields fromscripts/backfill-guild-settings.mjs.Rollout
/downloadon deploy; no changes required for music or other commands.Written for commit 7ea8269. Summary will update on new commits.
Summary by CodeRabbit
Removed Features
/downloadcommand and video/audio download functionality.Documentation
Maintenance