fix: eliminate mock state pollution in bot tests and remove resetMocks config

[LucasSantana-Dev]

Summary

Fixes coupled issues #1720 and #1633 by adding explicit mock cleanup and removing redundant Jest config.

Changes

Step 1: Fix mock state pollution (#1720)

File: packages/bot/src/functions/music/commands/play/handlers/postPlayBackgroundOps.spec.ts

Added explicit mock cleanup in beforeEach to prevent state pollution:

• Changed from relying on jest.clearAllMocks() to explicit mockClear() + re-establishment of default implementations
• Clears call history AND resets mock implementations for all mocked functions

Problem resolved: Tests that set .mockImplementation() or .mockRejectedValue() no longer leak state to subsequent tests. Previously, jest.clearAllMocks() did not clear implementations set during a test, causing two tests to fail:

• 'retries a transient applyStoredAutoplayPreference failure then succeeds' would get leaked 'clearAutoplayPause' in failedOps()
• 'records a breadcrumb when the preference op fails both attempts' would get leaked 'clearAutoplayPause' in failedOps()

Step 2: Remove redundant resetMocks config (#1633)

File: packages/bot/jest.config.cjs

Removed resetMocks: true (line 68) to match packages/backend/jest.config.cjs pattern (only clearMocks and restoreMocks).

Rationale: With explicit mock cleanup in place, resetMocks is redundant. restoreMocks is the correct Jest isolation mechanism.

Verification Results

✓ postPlayBackgroundOps.spec.ts: All 6 tests passing
✓ Test fix verified with resetMocks=false temporarily
✓ No ordering-dependent failures on multiple runs

Closes #1720
Closes #1633

---

Summary by cubic

Prevents mock state from leaking between bot tests by explicitly resetting mocks and removing Jest resetMocks. Also adds pnpm workspace allowBuilds: true flags for required packages. Addresses #1720 and #1633.

• Bug Fixes

  • postPlayBackgroundOps.spec.ts: clear mocks and reapply default implementations in beforeEach.
  • packages/bot/jest.config.cjs: remove resetMocks; keep clearMocks and restoreMocks.
  • lastFmSeeds.spec.ts: set getByDiscordId to resolve null for the no-user test.
  • queueManipulation.spec.ts: use jest.resetAllMocks() in beforeEach to drain queued .mockResolvedValueOnce() calls.

• Dependencies

  • Add pnpm-workspace.yaml with allowBuilds: true for @prisma/engines, msgpackr-extract, prisma, and unrs-resolver.

^{Written for commit cef6f08. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes

  • Improved test reliability by refining Jest mock setup/teardown for more deterministic, consistent behavior.
  • Updated mock clearing/resetting and restored mock handling to prevent state leakage between test cases.

• Chores

  • Updated workspace build settings for selected packages to better control build behavior in local and CI runs.

Step 3: full bot suite verification (added after initial PR)

The initial verification above only ran the single target spec file — the full bot suite was not checked before this PR was opened, despite restoreMocks being a repo-wide config change. Running the full suite surfaced 2 more ordering-dependent leaks the initial pass missed:

• lastFmSeeds.spec.ts — getByDiscordId's mocked resolved value leaked from a prior test into the "no cache or on error" case; fixed by explicitly setting it before the assertion instead of relying on leftover state.
• queueManipulation.spec.ts ("multi-user VC blend" block) — its beforeEach used jest.clearAllMocks(), which clears call history but does not drain queued .mockResolvedValueOnce() values from a prior test. Swapped to jest.resetAllMocks(), matching what the block's own pre-existing comment already assumed resetMocks: true was providing.

Full bot suite now green across 2 consecutive runs: 209/210 suites passed (1 pre-existing skip), 2852/2853 tests. Lint: 0 errors (69 pre-existing warnings). Typecheck: clean.

[coderabbitai]

📝 Walkthrough

Walkthrough

Switches the bot Jest config from resetMocks: true to restoreMocks: true, updates bot music test setup to reset and reinitialize specific mocks between cases, and adds allowBuilds permissions for selected workspace packages.

Changes

Jest mock lifecycle fix

Layer / File(s)	Summary
Jest config mock option change packages/bot/jest.config.cjs	Replaces resetMocks: true with restoreMocks: true.
Test mock isolation updates packages/bot/src/functions/music/commands/play/handlers/postPlayBackgroundOps.spec.ts, packages/bot/src/utils/music/autoplay/lastFmSeeds.spec.ts, packages/bot/src/utils/music/queueManipulation.spec.ts	Updates the affected specs to clear or reset specific mocks and set explicit return values for the exercised test paths.

pnpm Workspace Configuration

Layer / File(s)	Summary
Configure allowBuilds for workspace packages pnpm-workspace.yaml	Adds an allowBuilds block setting build permissions for @prisma/engines, msgpackr-extract, prisma, and unrs-resolver.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• LucasSantana-Dev/Lucky#378: Both PRs adjust Jest mock lifecycle/initialization in bot test suites to avoid ordering-dependent failures.
• LucasSantana-Dev/Lucky#513: Both PRs touch the same music autoplay/play test area and the mocks around those flows.
• LucasSantana-Dev/Lucky#1101: Both PRs modify the runPostPlayBackgroundOps test setup for the same background operations.

Suggested labels: ci

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The pnpm-workspace.yaml allowBuilds edits are unrelated to the linked Jest/mock-fix work and appear out of scope.	Remove the pnpm-workspace.yaml changes unless they are required for the mock-fix objectives and explicitly tied to a linked issue.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR addresses both linked issues by making postPlayBackgroundOps mocks explicit and replacing resetMocks with restoreMocks.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the two main changes: fixing bot test mock pollution and removing resetMocks from Jest config.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1633-1720-resetmocks-mock-pollution

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

	Warnings
⚠️	User-facing change without a CHANGELOG.md update. Add a line under ## [Unreleased] if this should appear in release notes. (Or apply the skip-changelog label if this PR does not affect end users.)

Generated by 🚫 dangerJS against cef6f08

[github-actions]

Failed to generate code suggestions for PR

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pnpm-workspace.yaml (1)

1-6: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Replace the placeholder allowBuilds values

pnpm-workspace.yaml still contains the autogenerated set this to true or false placeholders. Set each entry to true or false; as-is these packages stay unapproved and pnpm will skip their build scripts.

🤖 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 `@pnpm-workspace.yaml` around lines 1 - 6, The allowBuilds entries in
pnpm-workspace.yaml still use autogenerated placeholder text instead of real
boolean values. Update each package entry under allowBuilds to an explicit true
or false so pnpm can correctly approve or skip their build scripts. Use the
allowBuilds section to find the affected entries for `@prisma/engines`,
msgpackr-extract, prisma, and unrs-resolver.

🧹 Nitpick comments (1)

packages/bot/src/functions/music/commands/play/handlers/postPlayBackgroundOps.spec.ts (1)

51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

.mockClear() calls are redundant given global clearMocks: true.

clearMocks: true in packages/bot/jest.config.cjs already clears call history for all mocks (spies and plain jest.fn() alike) before every test, so the explicit .mockClear() calls here don't add anything beyond the .mockReturnValue()/.mockResolvedValue() reinitialization, which is what's actually needed. Harmless, but worth trimming for clarity.

Also applies to: 53-53, 55-55, 57-57, 59-59, 60-60

🤖 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/functions/music/commands/play/handlers/postPlayBackgroundOps.spec.ts`
at line 51, The explicit .mockClear() calls in postPlayBackgroundOps.spec.ts are
redundant because Jest’s global clearMocks setting already resets mock call
history before each test. Remove the unnecessary .mockClear() calls from the
test setup near clearAutoplayPause and the other listed mock initializations,
while keeping the .mockReturnValue() and .mockResolvedValue() setup for each
mock intact.

🤖 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.

Outside diff comments:
In `@pnpm-workspace.yaml`:
- Around line 1-6: The allowBuilds entries in pnpm-workspace.yaml still use
autogenerated placeholder text instead of real boolean values. Update each
package entry under allowBuilds to an explicit true or false so pnpm can
correctly approve or skip their build scripts. Use the allowBuilds section to
find the affected entries for `@prisma/engines`, msgpackr-extract, prisma, and
unrs-resolver.

---

Nitpick comments:
In
`@packages/bot/src/functions/music/commands/play/handlers/postPlayBackgroundOps.spec.ts`:
- Line 51: The explicit .mockClear() calls in postPlayBackgroundOps.spec.ts are
redundant because Jest’s global clearMocks setting already resets mock call
history before each test. Remove the unnecessary .mockClear() calls from the
test setup near clearAutoplayPause and the other listed mock initializations,
while keeping the .mockReturnValue() and .mockResolvedValue() setup for each
mock intact.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 70ca2bf7-67de-437e-ab93-b3d9183733a9

📥 Commits

Reviewing files that changed from the base of the PR and between 901e0fd and a2a0755.

📒 Files selected for processing (3)

• packages/bot/jest.config.cjs
• packages/bot/src/functions/music/commands/play/handlers/postPlayBackgroundOps.spec.ts
• pnpm-workspace.yaml

💤 Files with no reviewable changes (1)

• packages/bot/jest.config.cjs

[cubic-dev-ai]

All reported issues were addressed across 3 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 1 file (changes from recent commits).

<sub>Auto-approved: Test-only and config changes: removes redundant Jest resetMocks, adds explicit mock cleanup in tests to prevent state pollution, and adds pnpm build permissions. No production logic affected.

Re-trigger cubic</sub>

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud