Skip to content

test(shared): mutation-gate MusicControlService + GuildAutomationOrchestrator - #1454

Merged
LucasSantana-Dev merged 6 commits into
mainfrom
test/1447-1448-mutation-gate-batch2
Jun 15, 2026
Merged

LucasSantana-Dev merged 6 commits into
mainfrom
test/1447-1448-mutation-gate-batch2

Conversation

@LucasSantana-Dev

@LucasSantana-Dev LucasSantana-Dev commented Jun 15, 2026 •

Copy link
Copy Markdown
Owner

Second gate-expansion batch for sub-issues of #1426. Adds 2 services to the Stryker mutate set (now 17 modules gated).

Modules gated (scoped mutation total before → after)

Issue Module Before After
#1447 music/MusicControlService 14.17% 93.70%
#1448 guildAutomation/GuildAutomationOrchestrator 17.29% 79.84% (covered 90.35%)

Combined All files mutation score 83.23% → 83.67% (17 modules). break stays at 82 — kept ~1.7pt below combined for CI timing-variance headroom (orchestrator has a few timeout/errored mutants on the lock/cleanup paths). Full gate green locally.

Test-only + config change; no source touched. MusicControlService reaches 93.70%; the orchestrator jumps from 17%→79.84% total (90.35% of covered code killed) — remaining gap is a handful of timeout-prone lock-cleanup mutants, acceptable above the 82 gate.

Closes #1447
Closes #1448
Relates to #1426


Summary by cubic

Adds music/MusicControlService and guildAutomation/GuildAutomationOrchestrator to the Stryker mutate set and expands tests to gate mutants, raising scores to 93.70% and 90.70%. Hardens specs (locks/TTL, severity mapping, actualState precedence) and clears 4 CodeQL findings; test-only + config; gate stays 82 — completes #1447 and #1448 (part of #1426) and unblocks #1454.

  • Dependencies

    • Add src/services/music/MusicControlService.ts and src/services/guildAutomation/GuildAutomationOrchestrator.ts to packages/shared/stryker.conf.json mutate list.
  • Bug Fixes

    • Resolve CodeQL unused imports/vars and use safe fake timers in specs; tests now 95.

Written for commit bd67c19. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

Tests

  • Expanded unit test coverage for Guild Automation Orchestrator with new test helpers and scenarios covering plan creation, manifest operations, and lock management.
  • Substantially expanded unit test coverage for Music Control Service covering connection handling, command and result management, state subscriptions, and error cases.

Chores

  • Enhanced mutation testing configuration to include additional services in automated mutation testing.

50 tests; scoped mutation total 17.29% -> 79.84% (covered 90.35%).
Add music/MusicControlService and guildAutomation/GuildAutomationOrchestrator
to the Stryker mutate set. Combined All-files score 83.23% -> 83.67%
(17 modules). break stays 82 (headroom for CI timing variance).
@vercel

vercel Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lucky Ready Ready Preview, Comment Jun 15, 2026 7:28pm

Request Review

@LucasSantana-Dev
LucasSantana-Dev enabled auto-merge (squash) June 15, 2026 17:38

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Two spec files are substantially expanded with new test helpers and broad method-level coverage: GuildAutomationOrchestrator.spec.ts (~1361 lines added) and MusicControlService.spec.ts (~900 lines added). The Stryker configuration is updated to register both services as mutation targets.

Changes

GuildAutomationOrchestrator Test Expansion

Layer / File(s) Summary
Test helpers and mock infrastructure
packages/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts
Reworks Jest setup; introduces baseManifest, makeRepository (fully mocked IGuildAutomationRepository), makeOrchestrator (with injectable repo/updateRunStatus override), and stubPlan (bypasses the capture→diff→persist pipeline).
createApplyRun tests: status, blocking, lock lifecycle
packages/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts
Covers plan-only pending behavior, protected-operation blocking vs explicit allow, diagnostics contents, runType/allowProtected defaults, and lock lifecycle (release on success, concurrent rejection, TTL expiry via fake timers).
Manifest, capture, and run lifecycle method tests
packages/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts
Tests saveManifest (validation and option passthrough), getManifest (null when missing), recordCapture, markRunFailure (Error and plain object), completeRun (with/without diagnostics), updateRunStatus (all params and error case), and getStatus (with drifts).
runCutover, createPlan, and listRuns tests
packages/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts
Covers runCutover (blocked/complete/forced, missing manifest, optional parity fields), createPlan (missing manifest/capture errors, actualState precedence, runType defaulting, upsertDrift severity mapping, usedCapturedState diagnostics), and listRuns (default/custom limit, empty results).

MusicControlService Test Expansion and Stryker Config

Layer / File(s) Summary
Test setup, mocking, and helper builders
packages/shared/src/services/music/MusicControlService.spec.ts
Updates Jest mocks for logging, Sentry, ioredis, and Redis config; adds WithClients interface; replaces static builders with dynamic buildCommand and buildState helpers supporting partial overrides.
Health, connect, and disconnect tests
packages/shared/src/services/music/MusicControlService.spec.ts
Validates unhealthy/healthy readiness, null-client failure cases, dual-client connect with error logging, and subscriber unsubscribe/disconnect plus publisher disconnect with success/failure logging.
Command send/subscribe/result flow tests
packages/shared/src/services/music/MusicControlService.spec.ts
Expands sendCommand (unhealthy fast-fail, timeout cleanup, publish-rejection with Sentry capture), subscribeToCommands (channel filtering, handler invocation, error logging), sendResult, and subscribeToResults (result resolution, unknown-id handling, JSON parse errors).
State publish/subscribe/cache and createCommandId tests
packages/shared/src/services/music/MusicControlService.spec.ts
Adds publishState (Redis setex caching, error logging), subscribeToState (channel filtering, handler invocation), getState (cache retrieval, null/missing/rejection), and strengthened createCommandId regex/uniqueness assertions.
Stryker mutation targets config
packages/shared/stryker.conf.json
Extends the mutate array to include MusicControlService.ts and GuildAutomationOrchestrator.ts alongside the existing TwitchControlService.ts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • LucasSantana-Dev/Lucky#982: The expanded GuildAutomationOrchestrator.spec.ts adds unit tests for the orchestration behaviors (lock/TTL handling, cutover, plan creation, status updates) introduced by that PR's Orchestrator/Repository implementation.

Suggested labels

shared, size/xl

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding two services to mutation-test gates with expanded test coverage.
Linked Issues check ✅ Passed The PR fulfills all coding objectives from #1447 and #1448: expanded test coverage for both services, added modules to mutation gates in stryker.conf.json, and verified mutation scores.
Out of Scope Changes check ✅ Passed All changes are directly aligned with PR objectives: test expansions for MusicControlService and GuildAutomationOrchestrator, and configuration updates to stryker.conf.json to gate these modules.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 test/1447-1448-mutation-gate-batch2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

Failed to generate code suggestions for PR

@github-actions

github-actions Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Warnings
⚠️

Big PR — 2466 lines changed across 3 files. Consider splitting into smaller, reviewable chunks.

Generated by 🚫 dangerJS against bd67c19

Comment thread packages/shared/src/services/music/MusicControlService.spec.ts Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts`:
- Around line 1046-1080: The test for "uses provided actualState over
lastCapturedState" has a descriptive title but the assertion does not verify
that the provided state is actually being used. The current assertion only
checks that call[4] (operations array) has a length property, which does not
confirm that the providedState (with version 99 and guild.discordId
'provided-guild-id') took precedence over storedManifest or lastCapturedState.
Add assertions that examine the createPlanRecord mock call arguments to verify
that the provided state's unique properties were actually used in the plan
creation, such as checking that the plan diagnostics or arguments reflect the
version 99 and discordId 'provided-guild-id' from providedState, not the values
from storedManifest.
- Around line 1200-1231: The severity-mapping tests are only asserting that
upsertDrift was called, but not verifying the actual severity argument passed.
In
packages/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts
at lines 1200-1231, 1233-1253, 1255-1286, and 1288-1318, replace the
non-assertive expect(mocks.upsertDrift).toHaveBeenCalled() with an actual
assertion that checks the severity values collected in the severities array
match the expected severity for each test: 'low' for the 1-2 operations test,
'medium' for the corresponding mid-range operations test, and 'high' for the
high operations test. This ensures the tests will catch severity-mapping
regressions rather than just verifying the mock was invoked.
- Around line 220-238: The test uses jest.useFakeTimers() but restoration with
jest.useRealTimers() is not protected, so if any assertion fails before
restoration, fake timers will leak to subsequent tests. Restructure the test by
moving jest.useFakeTimers() before a try block, keeping all the test logic
(including all await calls and expect statements in the 'releases lock in
finally block when plan succeeds' test and the other affected tests) inside the
try block, and placing jest.useRealTimers() in a finally block to ensure timer
restoration always occurs regardless of whether assertions pass or fail.

In `@packages/shared/src/services/music/MusicControlService.spec.ts`:
- Around line 334-349: The test "uses default timeout of 10000ms" currently
waits on a real 10,000ms timeout when calling sendCommand, making the test slow.
Replace the real timeout with fake timers by calling jest.useFakeTimers() before
the test or within the test setup, then use jest.runAllTimers() or
jest.advanceTimersByTime() to simulate the timeout passage without actually
waiting. This will make the test run quickly while still verifying the 10-second
timeout behavior. Remember to restore real timers after the test completes using
jest.useRealTimers() if using fake timers at the test level.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5ae7412-c6b4-46cf-8ab1-a23ec4e8af8a

📥 Commits

Reviewing files that changed from the base of the PR and between 2bf3d93 and be0593f.

📒 Files selected for processing (3)
  • packages/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts
  • packages/shared/src/services/music/MusicControlService.spec.ts
  • packages/shared/stryker.conf.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: Test — bot
  • GitHub Check: Test — frontend
  • GitHub Check: Checks
  • GitHub Check: Test — backend
  • GitHub Check: Test — shared
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: quality / Lint (lint)
  • GitHub Check: quality / SAST (CodeQL) (javascript-typescript)
  • GitHub Check: Build — bot
  • GitHub Check: Build — backend
  • GitHub Check: Build — frontend
  • GitHub Check: Mutation — shared
🔇 Additional comments (1)
packages/shared/stryker.conf.json (1)

25-27: LGTM!

Comment thread packages/shared/src/services/music/MusicControlService.spec.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 3 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread packages/shared/src/services/guildAutomation/GuildAutomationOrchestrator.spec.ts Outdated
Comment thread packages/shared/src/services/music/MusicControlService.spec.ts Outdated
- orchestrator: drive createAutomationPlan via mock to assert real severity
  values (none/low/medium/high boundaries) instead of toHaveBeenCalled;
  raises module mutation 79.84% -> 90.70%.
- orchestrator: assert provided actualState precedence; protect fake timers
  with try/finally to prevent leakage.
- music: use fake timers for the 10s default-timeout test (no real wait).

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@LucasSantana-Dev

Copy link
Copy Markdown
Owner Author

@coderabbitai addressed all 4 findings in 16ccc29:

  1. Severity tests now mock createAutomationPlan to drive deterministic byModule counts and assert the actual severity arg to upsertDrift at every boundary (none/low/medium/high) — module mutation score rose 79.84%→90.70%.
  2. actualState-precedence test now asserts the provided state reached createAutomationPlan.
  3. Fake-timer lock tests wrapped in try/finally.
  4. Music default-timeout test uses fake timers (no real 10s wait).
    Full shared gate green at break:82 (combined 84.36%).

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@LucasSantana-Dev: All four findings confirmed addressed — great work on the follow-up:

  1. Severity boundary tests — mocking createAutomationPlan for deterministic byModule counts and asserting the exact severity arg at every boundary is the right approach; the 79.84%→90.70% score jump validates it.
  2. actualState-precedence assertion — confirming the provided state actually flows through to createAutomationPlan closes the gap properly.
  3. try/finally on fake-timer lock tests — important guard to prevent timer leakage between tests.
  4. Music default-timeout with fake timers — eliminates the flaky real-time wait cleanly.

Combined 84.36% with break:82 gives a comfortable 2.36pp buffer.

(๑˃ᴗ˂)ﻭ


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 15, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Requires human review: Auto-approving this large, test-only expansion is risky. A human review is needed to verify coverage quality and test correctness.

Re-trigger cubic

LucasSantana-Dev added a commit that referenced this pull request Jun 15, 2026
…#1457)

Closes #1456.

## Problem

The `Security` CI check (`npm audit --audit-level=high`) fails on
**every open PR** (e.g. #1454, #1455) — the lockfile pins two transitive
deps with high-severity advisories, surfaced by an advisory-DB update.
This blocks the merge queue (priority #3).

## Fix

| package | before | after | advisory |
|---|---|---|---|
| `vite` | 8.0.14 | 8.0.16 | GHSA-fx2h-pf6j-xcff (`server.fs.deny`
bypass), GHSA-v6wh-96g9-6wx3 (launch-editor NTLMv2) |
| `form-data` | 4.0.5 | 4.0.6 | CRLF injection via unescaped multipart
field names |

- `packages/frontend/package.json`: vite devDependency floor `^8.0.14` →
`^8.0.16`. vite 8.0.16 requires rolldown 1.0.3, which accounts for the
bulk of the lockfile churn.
- `package.json`: root `overrides` pin `form-data: ">=4.0.6"`
(transitive, no workspace owner).
- `package-lock.json`: regenerated; `npm audit --audit-level=high` → **0
high** (verified locally).

Both are dev/build-tool deps not shipped to the bot runtime. All four
workspace `type:check`s pass locally with vite 8.0.16. Moderate
advisories remain below the gate and are out of scope.

## Verification

- [x] `npm audit --audit-level=high` exits 0
- [x] `npm run type:check` (shared, bot, backend, frontend) passes
- [ ] CI: Build/Test (all packages), Security

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
  * Updated build tool dependency to latest compatible version.
* Extended dependency override configurations for enhanced
compatibility.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Bump `vite` to 8.0.16 and enforce `form-data` >= 4.0.6 to clear
high-severity advisories and unblock the Security CI gate. Also pin
`vite` at the repo root so `@vitejs/plugin-react` resolves it correctly
during builds.

- **Dependencies**
- Root dev + frontend: `vite` ^8.0.16 (was ^8.0.14); pulls `rolldown`
1.0.3 and `tinyglobby` ^0.2.17.
- Root overrides: `form-data` >= 4.0.6 to fix CRLF injection in
transitive deps.
- Regenerated lockfile; `npm audit --audit-level=high` now reports 0
high.

- **Bug Fixes**
- Fixed hoisting so `@vitejs/plugin-react` resolves the updated `vite`;
`npm run build:frontend` succeeds.

<sup>Written for commit dba6347.
Summary will update on new commits.</sup>

<a
href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1457?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. -->

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Clears 4 CodeQL findings blocking #1454: unused afterEach import in
MusicControlService.spec, unused makeRepository destructuring and
unused result binding in GuildAutomationOrchestrator.spec. Specs still
pass (95 tests).

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Auto-approved: Test-only and config changes adding mutation testing for two services. No source logic modified; low risk of breakage.

Re-trigger cubic

@sonarqubecloud

Copy link
Copy Markdown

@LucasSantana-Dev
LucasSantana-Dev merged commit ae478ec into main Jun 15, 2026
45 checks passed
@LucasSantana-Dev
LucasSantana-Dev deleted the test/1447-1448-mutation-gate-batch2 branch June 15, 2026 19:31

This branch was successfully deployed

1 active deployment
Preview — bd67c191 Deployed Jun 15, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand tests then mutation-gate GuildAutomationOrchestrator Expand tests then mutation-gate MusicControlService

2 participants