feat(asset-gen): add MiniMax music cover support - #1325
Open
octo-patch wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughMiniMax music-cover generation is added to the audio asset-generation flow. The change adds request fields, server forwarding, provider registration, model catalog entries, regional API handling, validation, polling, tests, and documentation. ChangesMiniMax audio generation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as generate_audio
participant Unity as GenerateAudio
participant Adapter as MiniMaxAudioAdapter
participant API as MiniMax API
Client->>Server: submit cover parameters
Server->>Unity: forward audio payload
Unity->>Adapter: create cover request
Adapter->>API: POST authenticated JSON request
API-->>Adapter: status and audio result
Adapter-->>Unity: provider poll result
Unity-->>Client: pending job or imported AudioClip
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@MCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs`:
- Around line 92-95: Update the shared download policy or transport used by
AssetGenJobManager.IsAllowedDownloadUrl to validate resolved hosts and reject
loopback, link-local, private-network, and other non-public addresses, while
also preventing redirects to disallowed destinations. Ensure the
MiniMaxAudioAdapter assignment to _downloadUrl cannot initiate a request until
the complete URL and redirect chain pass these checks.
- Around line 180-185: The audio pipeline must not expose raw pcm files as
importable results. In
MCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs:180-185,
update NormalizeAudioFormat to reject pcm unless conversion to a supported
container is implemented; in
MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs:446, remove pcm from
AudioAllowedExtensions unless a custom importer exists; and in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MiniMaxAudioAdapterTests.cs:127-146,
add end-to-end import coverage for every remaining accepted audio format.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateAudioTests.cs`:
- Line 23: Restore the previous MCPFORUNITY_MINIMAX_API_KEY value after each
test that clears it. Update the test class’s TearDown method to save the
original environment value before modification and restore it afterward,
preserving whether the variable was initially unset.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ed8f358-a147-448d-bff5-2f8196e17a5e
📒 Files selected for processing (19)
MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.csMCPForUnity/Editor/Services/AssetGen/AssetGenModelCatalog.csMCPForUnity/Editor/Services/AssetGen/Providers/AssetGenProviders.csMCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.csMCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs.metaMCPForUnity/Editor/Services/AssetGen/Providers/ProviderModels.csMCPForUnity/Editor/Tools/AssetGen/GenerateAudio.csMCPForUnity/Editor/Windows/Components/AssetGen/McpAssetGenSection.csServer/src/cli/commands/asset_gen.pyServer/src/services/tools/generate_audio.pyServer/tests/test_asset_gen_audio.pyTestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenModelCatalogTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenProvidersTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateAudioTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MiniMaxAudioAdapterTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MiniMaxAudioAdapterTests.cs.metawebsite/docs/reference/tools/asset_gen/generate_audio.mdwebsite/docs/reference/tools/asset_gen/index.mdwebsite/docs/reference/tools/index.md
Comment on lines
+92
to
+95
| if (Uri.TryCreate(audio, UriKind.Absolute, out Uri audioUri) | ||
| && (audioUri.Scheme == Uri.UriSchemeHttp || audioUri.Scheme == Uri.UriSchemeHttps)) | ||
| { | ||
| _downloadUrl = audio; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Restrict provider-returned download URLs before use.
audio can contain any HTTP(S) URL. AssetGenJobManager.IsAllowedDownloadUrl checks only the scheme. It permits loopback, link-local, and private-network hosts.
A compromised or malicious provider response can make the Unity Editor request internal services. Enforce private-address and redirect validation in the shared transport or download policy before the request starts.
🤖 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 `@MCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs` around
lines 92 - 95, Update the shared download policy or transport used by
AssetGenJobManager.IsAllowedDownloadUrl to validate resolved hosts and reject
loopback, link-local, private-network, and other non-public addresses, while
also preventing redirects to disallowed destinations. Ensure the
MiniMaxAudioAdapter assignment to _downloadUrl cannot initiate a request until
the complete URL and redirect chain pass these checks.
Comment on lines
+180
to
+185
| private static string NormalizeAudioFormat(string value) | ||
| { | ||
| string normalized = string.IsNullOrWhiteSpace(value) ? "mp3" : value.Trim().ToLowerInvariant(); | ||
| if (normalized != "mp3" && normalized != "wav" && normalized != "pcm") | ||
| throw new ArgumentException("audio_format must be 'mp3', 'wav', or 'pcm'."); | ||
| return normalized; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'ProjectVersion.txt' .
fd -a -a 'AudioImportPipeline.cs' MCPForUnity TestProjects
rg -n -C 5 --glob '*.cs' 'pcm|AudioImportPipeline|AudioClip|AssetImporter' MCPForUnity TestProjectsRepository: CoplayDev/unity-mcp
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- MiniMaxAudioAdapter ---'
sed -n '1,240p' MCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs
printf '%s\n' '--- AssetGenJobManager write/import flow ---'
sed -n '120,190p' MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs
sed -n '390,520p' MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs
printf '%s\n' '--- AudioImportPipeline ---'
cat -n MCPForUnity/Editor/Services/AssetGen/Import/AudioImportPipeline.cs
printf '%s\n' '--- Audio-related request/result models and tests ---'
rg -n -C 4 --glob '*.cs' 'class .*Audio|AudioFormat|OutputFormat|ResultExt|InlineData|OverrideExt|IsAllowedResultExtension|ImportOverrideForTests' MCPForUnity/Editor/Services/AssetGen TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGenRepository: CoplayDev/unity-mcp
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Runner poll/download/import state transitions ---'
sed -n '300,438p' MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs
printf '%s\n' '--- MiniMax adapter tests ---'
cat -n TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MiniMaxAudioAdapterTests.cs
printf '%s\n' '--- Audio pipeline tests ---'
cat -n TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AudioImportPipelineTests.cs
printf '%s\n' '--- Extension allowlist tests and audio test helpers ---'
rg -n -C 8 --glob '*.cs' 'IsAllowedResultExtension|StartAudioGeneration|ImportOverrideForTests|OutputFormat|AudioFormat|MiniMax' TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen MCPForUnity/Editor/Services/AssetGenRepository: CoplayDev/unity-mcp
Length of output: 50376
🌐 Web query:
Unity Manual audio file compatibility raw PCM .pcm supported import formats AudioImporter
💡 Result:
Unity does not support the import of raw PCM (.pcm) files directly [1][2][3]. While "PCM" is a widely supported compression format within Unity for runtime audio storage and playback, this refers to an uncompressed encoding method that Unity can apply to audio files during the import and transcoding process [4][5]. It is not a supported source file format for the AudioImporter [3][6]. The official supported source audio file formats that can be imported into Unity are: - Microsoft Wave (.wav) [1][2][3] - Audio Interchange File Format (.aiff,.aif) [1][2][3] - MPEG layer 3 (.mp3) [1][2][3] - Ogg Vorbis (.ogg) [1][2][3] - Free Lossless Audio Codec (.flac) [1][2][3] - Tracker modules (.mod,.it,.s3m,.xm) [7][2][3] When you import any of these supported files, the Unity AudioImporter allows you to specify the runtime compression format, where "PCM" can be selected to ensure the audio remains uncompressed for optimal CPU performance at the cost of larger file size [4][5]. Raw PCM data lacking a container format (like a.wav header) cannot be natively processed by the AudioImporter [3][6].
Citations:
- 1: https://docs.unity3d.com/Manual/AudioFiles-compatibility.html
- 2: https://docs.unity3d.com/6000.6/Documentation/Manual/AudioFiles-compatibility.html
- 3: https://docs.unity3d.com/Manual/assets-supported-types.html
- 4: https://docs.unity3d.com/Manual/class-AudioClip.html
- 5: https://docs.unity3d.com/Manual/AudioFiles-compression.html
- 6: https://docs.unity3d.com/2023.2/Documentation/Manual/BuiltInImporters.html
- 7: https://docs.unity3d.com/2023.1/Documentation/Manual/AudioFiles.html
Do not expose raw pcm as an importable audio result.
The job writes pcm bytes as .pcm under Assets/, but Unity does not import raw PCM source files. The bytes lack container metadata such as sample rate and channel count. The job can therefore fail or produce no usable AudioClip.
- Reject
pcmunless the pipeline converts it to a supported container. - Remove
pcmfromAudioAllowedExtensionsunless a custom PCM importer exists. - Add end-to-end import coverage for each accepted audio format.
📍 Affects 3 files
MCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs#L180-L185(this comment)MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs#L446-L446TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MiniMaxAudioAdapterTests.cs#L127-L146
🤖 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 `@MCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs` around
lines 180 - 185, The audio pipeline must not expose raw pcm files as importable
results. In
MCPForUnity/Editor/Services/AssetGen/Providers/MiniMaxAudioAdapter.cs:180-185,
update NormalizeAudioFormat to reject pcm unless conversion to a supported
container is implemented; in
MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs:446, remove pcm from
AudioAllowedExtensions unless a custom importer exists; and in
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MiniMaxAudioAdapterTests.cs:127-146,
add end-to-end import coverage for every remaining accepted audio format.
| { | ||
| AssetGenJobManager.ResetForTests(); | ||
| Environment.SetEnvironmentVariable("MCPFORUNITY_FAL_API_KEY", null); | ||
| Environment.SetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY", null); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restore MCPFORUNITY_MINIMAX_API_KEY after each test.
Line 23 clears a process-wide environment variable. TearDown does not restore its prior value. Later tests can observe the changed environment and produce order-dependent results.
Proposed fix
private string _dir;
private EncryptedFileKeyStore _store;
+private string _originalMiniMaxApiKey;
[SetUp]
public void SetUp()
{
AssetGenJobManager.ResetForTests();
Environment.SetEnvironmentVariable("MCPFORUNITY_FAL_API_KEY", null);
+ _originalMiniMaxApiKey = Environment.GetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY");
Environment.SetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY", null);
@@
public void TearDown()
{
AssetGenJobManager.ResetForTests();
SecureKeyStore.ResetForTests();
+ Environment.SetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY", _originalMiniMaxApiKey);
try { if (Directory.Exists(_dir)) Directory.Delete(_dir, true); } catch { }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Environment.SetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY", null); | |
| private string _dir; | |
| private EncryptedFileKeyStore _store; | |
| private string _originalMiniMaxApiKey; | |
| [SetUp] | |
| public void SetUp() | |
| { | |
| AssetGenJobManager.ResetForTests(); | |
| Environment.SetEnvironmentVariable("MCPFORUNITY_FAL_API_KEY", null); | |
| _originalMiniMaxApiKey = Environment.GetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY"); | |
| Environment.SetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY", null); | |
| } | |
| public void TearDown() | |
| { | |
| AssetGenJobManager.ResetForTests(); | |
| SecureKeyStore.ResetForTests(); | |
| Environment.SetEnvironmentVariable("MCPFORUNITY_MINIMAX_API_KEY", _originalMiniMaxApiKey); | |
| try { if (Directory.Exists(_dir)) Directory.Delete(_dir, true); } catch { } | |
| } |
🤖 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
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateAudioTests.cs`
at line 23, Restore the previous MCPFORUNITY_MINIMAX_API_KEY value after each
test that clears it. Update the test class’s TearDown method to save the
original environment value before modification and restore it afterward,
preserving whether the variable was initially unset.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason: The audio generation tool cannot create MiniMax covers from reference audio or parse cover responses.
Changes
Checks
cd Server && ./.venv/bin/python -m pytest tests/test_asset_gen_audio.py -qcd Server && ./.venv/bin/python ../tools/generate_docs_reference.py --checkgit diff --checkSummary by CodeRabbit
New Features
Documentation
Tests