Skip to content

feat(asset-gen): AI audio generation + shared model panel (hardened)#1264

Merged
Scriptwonder merged 11 commits into
CoplayDev:betafrom
Scriptwonder:feat/assetgen-audio-model-panel
Jul 13, 2026
Merged

feat(asset-gen): AI audio generation + shared model panel (hardened)#1264
Scriptwonder merged 11 commits into
CoplayDev:betafrom
Scriptwonder:feat/assetgen-audio-model-panel

Conversation

@Scriptwonder

@Scriptwonder Scriptwonder commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds AI audio generation to the Asset Generation feature and a shared model control panel across image / 3D / audio, then hardens the whole subsystem after a security audit + code review.

  • generate_audio MCP tool + CLI — fal.ai audio (Stable Audio 2.5 default, CassetteAI SFX/Music, Google Lyria 2), reusing the single fal key. Backend: FalAudioAdapter, AudioImportPipeline (load type branches on clip length), AssetGenJobManager.StartAudioGeneration.
  • Model control panel — a curated AssetGenModelCatalog (per-model use-case / price / duration / license metadata) drives per-provider Model dropdowns on the image/3D/audio rows, a fal-audio row (no key field — shares the fal key), and a Refresh button. Model prefs are per-(kind, provider).

Security hardening (from a dynamic security audit)

  • Provider API-key exfiltration via redirectUnityWebRequestTransport now sets redirectLimit=0 on any request carrying an Authorization header, so a provider 3xx can't re-send the key to a redirect host.
  • Provider-controlled file extension → Editor RCE — a per-kind result-extension allowlist at the write/import boundary (AssetGenJobManager.WriteFile + defense-in-depth in the audio/image import pipelines). A hostile provider response can no longer land a .cs/.asmdef/.meta/.asset under Assets/ and get it compiled on refresh.
  • response_url / model host-pinningProviderHttp.RequireHost validates the submit URL and the provider-supplied response_url are https://queue.fal.run before the fal key is ever attached (both fal image + audio adapters).

Correctness fixes (from code review)

  • Tripo image→3D now honors the selected model_version.
  • FalAudioAdapter.BuildBody is catalog-driven: duration-required models (CassetteAI SFX/Music, Stable Audio) send a sensible default duration when the caller passes none (fixes a default-input fal 422), fractional durations floor to ≥1, Lyria stays prompt-only and its GUI no longer advertises a duration it ignores, and per-model clamp ceilings come from the catalog (no duplicated constants).
  • Unknown fal poll status fails fast instead of polling to the 600s timeout.
  • Stale/invalid selected-model pref is cleared on dropdown fallback; the audio fal-key status refreshes when the shared 2D fal key changes.

Cleanup

  • AssetGenModelCatalog.ResolveModel(...) dedupes the model-resolution chain across the three generate tools.
  • Tool-group label fix: asset_gen"Asset Gen" (was falling back to "Asset_gen").
  • Reference docs regenerated from the Python registry (adds generate_audio.md, refreshes the Asset Gen landscape).

Testing

  • EditMode: 1166 tests, 0 failures (+19 new regression tests covering every fix above), verified on the 2021.3 floor.
  • Python: 34 asset-gen tests pass. Docs --check green. Compile matrix (pre-push hook) green.
  • Live fal smoke tests run in-editor: SFX (3s), CassetteAI Music (20s), Stable Audio (30s) — all import with the expected specs; the label fix verified live (asset_gen → "Asset Gen").

Notes for reviewers

  • Draft: the 6 LOW security findings (download-host SSRF guard, image_url validation, download size cap, request timeout/abort, concurrency cap, job-record accumulation) are not in this PR — several are pre-existing shared-infra gaps; happy to split them into a follow-up.
  • Release notes / Recent Updates are auto-generated from the published GitHub Release, so this PR description doubles as the v10.1 change summary.

🤖 Prepared with Claude Code.

https://claude.ai/code/session_015KYy51gwBuhDuLZXXoqc98

Summary by CodeRabbit

  • New Features
    • Added audio generation support end-to-end (generate/status/cancel/list providers) with provider + model selection and automatic Unity AudioClip import.
    • Introduced curated model selection with per-provider saved preferences and updated Asset Generation UI (audio rows, model dropdowns, refresh).
    • Added generate_audio CLI/MCP tool support and refreshed tool documentation.
  • Bug Fixes
    • Strengthened validation for imported result file types and improved safety when authenticated (prevents redirect behavior) and when provider URLs are untrusted.
  • Documentation
    • Updated and added audio tool reference pages.
  • Tests
    • Added/expanded unit and integration tests covering audio flows, model catalog selection, validation/allowlists, and security edge cases.

…nager entry

Phase 1 of fal.ai audio generation. Adds AudioGenRequest + IAudioProviderAdapter,
a FalAudioAdapter fronting the v1 fal audio models (stable-audio-25, cassetteai/*,
lyria2), an AudioImportPipeline that branches AudioImporter load type on clip
length, AssetGenProviders.Audio + an audio List row, and
AssetGenJobManager.StartAudioGeneration. Also threads ModelGenRequest.Model
(consumed in Phase 3). Compiles on the 2021.3 floor; adapter/import/e2e tests green.

Claude-Session: https://claude.ai/code/session_01GCxmdd4qo7MG6J4M6WcT9Y
…wiring

Phases 2-3. Adds a curated AssetGenModelCatalog (image/3D/audio models with
price/duration/use-case metadata; defaults reference the adapter constants so the
panel default equals what an omitted model resolves to). Adds the generate_audio
MCP tool + CLI, threads a per-(kind,provider) selected-model pref so a GUI choice
becomes the default generate_* uses when no model is passed, and makes
Tripo/Meshy consume req.Model. Per-provider prefs (not per-type) so disjoint model
lists never clobber. Compiles on 2021.3 floor; Python + adapter/catalog tests green.

Claude-Session: https://claude.ai/code/session_01GCxmdd4qo7MG6J4M6WcT9Y
Phases 4-5. Extends the existing Asset Gen tab (no new window): per-provider
'Model' dropdowns on the image + 3D rows with price/duration/use-case metadata, a
fal audio row (no key field — reuses the shared fal key) with a model dropdown and
the Stable Audio license caveat, and a Refresh button that re-validates key
presence + the curated catalog. Selecting a model writes the per-(kind,provider)
pref that generate_* reads as its default. fal has no public list-models API, so
Refresh is a curated re-validate (the plan's gated fallback), never a network fetch.
Characterization tests assert the new builders + three-phase lifecycle.

Claude-Session: https://claude.ai/code/session_01GCxmdd4qo7MG6J4M6WcT9Y
The provider row is a vertical (column) container, so the setting-dropdown-inline
class's flex-grow:1 stretched the DropdownField vertically. Wrap the Model dropdown
in a horizontal .setting-row with a .setting-label (matching the Format row) and use
a label-less DropdownField. Verified live: all 5 model dropdowns now render at 18px
inside setting-row containers instead of ~400px boxes.

Claude-Session: https://claude.ai/code/session_01GCxmdd4qo7MG6J4M6WcT9Y
Visible tab label only; the internal assetgen-tab id (and all C# wiring) is unchanged.

Claude-Session: https://claude.ai/code/session_01GCxmdd4qo7MG6J4M6WcT9Y
Wraps each category (3D Models / 2D Images / Sound) in its own darker rounded panel
so they read as distinct blocks, and moves the per-provider key status (saved/not set)
up into the header row to the right of the provider name — reclaiming a line per
provider. Verified live: 3 panels (bg alpha 0.20), status inline in each header.

Claude-Session: https://claude.ai/code/session_01GCxmdd4qo7MG6J4M6WcT9Y
Security (from a dynamic security audit of the branch):
- H1: UnityWebRequestTransport disables auto-redirect on auth-bearing
  requests (redirectLimit=0) so a provider 3xx can't re-send the API key
  to a redirect host.
- H2/P8: per-kind result-extension allowlist in AssetGenJobManager.WriteFile
  (+ defense-in-depth in the audio/image import pipelines) — a provider
  can no longer land a .cs/.asmdef/.meta/.asset under Assets/ (Editor RCE).
- H3: ProviderHttp.RequireHost pins the submit URL and the provider-supplied
  response_url to https://queue.fal.run before the fal key is attached
  (both fal image + audio adapters).

Correctness (from code review):
- C1: Tripo image->3D now sends model_version.
- C2/C3/C6/C10: FalAudioAdapter.BuildBody is catalog-driven — duration-
  required models (CassetteAI SFX/Music, Stable Audio) send a default
  duration when the caller passes 0 (fixes the default-input 422),
  fractional durations floor to >=1, Lyria stays prompt-only and its GUI
  no longer advertises a duration it ignores, and the clamp ceilings come
  from the catalog (no more duplicated 190/30/180).
- C4: an unmapped fal poll status now fails fast instead of polling to the
  600s timeout (both fal adapters).
- C5: a stale/invalid selected-model pref is cleared on dropdown fallback.
- C7: the audio fal-key status refreshes when the shared 2D fal key changes.

Cleanup: extract AssetGenModelCatalog.ResolveModel (dedupes the model-
resolution chain across the three generate tools) + DefaultModelId no-alloc.

Verified: full EditMode suite 1166 tests, 0 failures (+19 new regression
tests); 34 Python asset-gen tests pass.

Claude-Session: https://claude.ai/code/session_015KYy51gwBuhDuLZXXoqc98
… docs

- McpToolsSection: add the asset_gen -> "Asset Gen" group display name so
  the tool-group tab no longer falls back to the raw "Asset_gen".
- Update the asset_gen group blurb (registry + CLI) to include audio gen.
- Regenerate the tool reference docs from the Python registry: add the
  missing generate_audio.md and refresh the asset_gen landscape/index.

Claude-Session: https://claude.ai/code/session_015KYy51gwBuhDuLZXXoqc98
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Audio generation is added across Unity and the server through fal adapters, curated model selection, asynchronous jobs, secure importing, editor controls, CLI/MCP commands, documentation, and tests. Existing image/model generation now resolves models through the catalog.

Changes

Asset generation expansion

Layer / File(s) Summary
Model and request contracts
MCPForUnity/Editor/Constants/*, MCPForUnity/Editor/Helpers/*, MCPForUnity/Editor/Services/AssetGen/AssetGenModelCatalog.cs, MCPForUnity/Editor/Services/AssetGen/Providers/ProviderModels.cs
Adds per-provider model preferences, audio request data, curated model metadata, and model resolution precedence.
Provider adapters and transport security
MCPForUnity/Editor/Services/AssetGen/Providers/*, MCPForUnity/Editor/Services/AssetGen/Http/*
Adds fal audio submission/polling, selected-model support, host validation, secret redaction, and authenticated redirect prevention.
Unity jobs and importing
MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs, MCPForUnity/Editor/Services/AssetGen/Import/*
Adds audio jobs, result-extension allowlists, audio importer settings, and invalid-result rejection.
Unity tools and editor controls
MCPForUnity/Editor/Tools/AssetGen/*, MCPForUnity/Editor/Windows/Components/AssetGen/*
Adds audio command handling, shared job responses, model resolution, provider filtering, model dropdowns, refresh behavior, and updated editor labels.
Server, documentation, and tests
Server/src/*, Server/tests/*, TestProjects/.../AssetGen/*, website/docs/reference/tools/*
Adds MCP/CLI audio surfaces, model forwarding, reference documentation, and coverage for provider, security, import, catalog, and routing behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCPClient
  participant ServerGenerateAudio
  participant UnityGenerateAudio
  participant AssetGenJobManager
  participant FalAudioAdapter
  participant AudioImportPipeline

  MCPClient->>ServerGenerateAudio: generate_audio(action, prompt, model)
  ServerGenerateAudio->>UnityGenerateAudio: send generate_audio command
  UnityGenerateAudio->>AssetGenJobManager: StartAudioGeneration(request)
  AssetGenJobManager->>FalAudioAdapter: submit and poll audio job
  FalAudioAdapter-->>AssetGenJobManager: result URL and extension
  AssetGenJobManager->>AudioImportPipeline: import downloaded audio
  AudioImportPipeline-->>UnityGenerateAudio: completed job status
  UnityGenerateAudio-->>ServerGenerateAudio: pending or completed response
  ServerGenerateAudio-->>MCPClient: structured result
Loading

Possibly related PRs

  • CoplayDev/unity-mcp#1218: Extends the shared AssetGen foundation with audio generation and per-kind/provider model selection.

Suggested reviewers: dsarno

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is useful, but it does not follow the repository template and omits required sections like Type of Change, Compatibility, and Related Issues. Rewrite it using the repository template, filling in the required sections for type of change, changes made, compatibility, testing, docs, related issues, and notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 8.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main addition of AI audio generation and the shared model panel.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@Scriptwonder Scriptwonder added the enhancement New feature or request label Jul 13, 2026
@Scriptwonder Scriptwonder requested a review from Copilot July 13, 2026 07:32
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Scriptwonder Scriptwonder marked this pull request as ready for review July 13, 2026 07:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR expands the Asset Generation subsystem by adding a new AI audio generation tool (fal.ai), introducing a shared per-provider model selection panel across image/3D/audio, and applying multiple security hardening measures around HTTP key handling and file import/write boundaries.

Changes:

  • Added generate_audio MCP tool + CLI command, plus Unity-side job flow (adapter + import pipeline) for fal.ai audio generation.
  • Introduced a curated AssetGenModelCatalog and per-(kind, provider) model preferences, wired into the Asset Gen UI and generate_image / generate_model / generate_audio.
  • Hardened transport and import boundaries (redirect handling for auth requests, host pinning, and per-kind result-extension allowlists), with additional regression tests.

Reviewed changes

Copilot reviewed 51 out of 55 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
website/docs/reference/tools/index.md Updates tools index to include generate_audio under asset_gen.
website/docs/reference/tools/asset_gen/index.md Updates asset_gen tool listing to include generate_audio.
website/docs/reference/tools/asset_gen/generate_model.md Documents new model parameter for generate_model.
website/docs/reference/tools/asset_gen/generate_audio.md Adds generated reference doc for generate_audio.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs Adds reflection-based characterization for AssetGen UI builders and refresh elements.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/UnityWebRequestTransportTests.cs.meta Adds Unity meta for new transport test.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/UnityWebRequestTransportTests.cs Tests redirect/auth decision helper (CarriesAuth).
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/TripoAdapterTests.cs Adds regression tests for honoring model version in Tripo adapter.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MeshyAdapterTests.cs Adds regression tests for honoring model selection in Meshy adapter.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateImageTests.cs Adds tests ensuring GUI-selected model pref is used when model param omitted.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateAudioTests.cs.meta Adds Unity meta for new audio tool tests.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateAudioTests.cs Adds C# tool tests for generate_audio behavior and provider listing.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/FalAudioAdapterTests.cs.meta Adds Unity meta for Fal audio adapter tests.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/FalAudioAdapterTests.cs Adds extensive tests for fal audio submit/poll, duration behavior, and host pinning.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/FalAdapterTests.cs Adds tests for unknown poll status fail-fast and host pinning for fal image adapter.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AudioImportPipelineTests.cs.meta Adds Unity meta for audio import pipeline tests.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AudioImportPipelineTests.cs Adds guard-path tests for audio import pipeline (Assets path + extension allowlist).
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenProvidersTests.cs Adds tests for audio provider adapter factory + provider list shape.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenPrefsTests.cs Adds tests for per-(kind, provider) selected model prefs and default audio provider.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenModelCatalogTests.cs.meta Adds Unity meta for model catalog tests.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenModelCatalogTests.cs Adds tests for curated catalog contents, defaults, and drift-guard expectations.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenJobManagerTests.cs Adds audio end-to-end job tests and extension allowlist tests.
Server/tests/test_asset_gen_model.py Updates Python tests to allow/pass through model param for generate_model.
Server/tests/test_asset_gen_audio.py Adds Python tests for generate_audio tool + CLI pass-through and key-safety constraints.
Server/src/services/tools/generate_model.py Adds model parameter and forwards it to Unity.
Server/src/services/tools/generate_audio.py Implements Python generate_audio MCP tool and forwards params to Unity.
Server/src/services/registry/tool_registry.py Updates asset_gen group description to include audio.
Server/src/cli/commands/asset_gen.py Adds --model to model gen CLI and introduces generate-audio CLI command.
MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml Renames Asset Gen tab label to “Generative”.
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs Fixes asset_gen tool group label mapping to “Asset Gen”.
MCPForUnity/Editor/Windows/Components/AssetGen/McpAssetGenSection.uxml Adds Refresh button and status label to AssetGen panel UI.
MCPForUnity/Editor/Windows/Components/AssetGen/McpAssetGenSection.cs Adds model dropdown UI, audio row, refresh behavior, and UI restructuring into category panels.
MCPForUnity/Editor/Tools/AssetGen/GenerateModel.cs Resolves selected model via catalog/prefs and filters provider list to model kind.
MCPForUnity/Editor/Tools/AssetGen/GenerateImage.cs Resolves selected model via catalog/prefs for image generation.
MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs.meta Adds Unity meta for new audio tool.
MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs Implements Unity-side generate_audio tool (generate/status/cancel/list_providers).
MCPForUnity/Editor/Services/AssetGen/Providers/TripoAdapter.cs Adds request-level model_version support and exposes default version for catalog.
MCPForUnity/Editor/Services/AssetGen/Providers/ProviderModels.cs Adds AudioGenRequest and extends ProviderInfo.Kind to include audio.
MCPForUnity/Editor/Services/AssetGen/Providers/ProviderHttp.cs Adds host-pinning helper (RequireHost) with key scrubbing.
MCPForUnity/Editor/Services/AssetGen/Providers/OpenRouterAdapter.cs Exposes default model for catalog.
MCPForUnity/Editor/Services/AssetGen/Providers/MeshyAdapter.cs Adds request-level model selection and ensures refine uses the same model.
MCPForUnity/Editor/Services/AssetGen/Providers/IProviderAdapters.cs Adds IAudioProviderAdapter interface.
MCPForUnity/Editor/Services/AssetGen/Providers/FalAudioAdapter.cs.meta Adds Unity meta for new fal audio adapter.
MCPForUnity/Editor/Services/AssetGen/Providers/FalAudioAdapter.cs Implements fal queue API audio adapter (submit/poll, duration handling, host pinning).
MCPForUnity/Editor/Services/AssetGen/Providers/FalAdapter.cs Adds host pinning and fail-fast behavior on unknown poll statuses for images.
MCPForUnity/Editor/Services/AssetGen/Providers/AssetGenProviders.cs Adds audio provider factory + list row for fal audio.
MCPForUnity/Editor/Services/AssetGen/Import/ImageImportPipeline.cs Adds defense-in-depth extension check before importing images.
MCPForUnity/Editor/Services/AssetGen/Import/AudioImportPipeline.cs.meta Adds Unity meta for new audio import pipeline.
MCPForUnity/Editor/Services/AssetGen/Import/AudioImportPipeline.cs Adds audio import pipeline with per-extension guard and load-type selection by clip length.
MCPForUnity/Editor/Services/AssetGen/Http/UnityWebRequestTransport.cs Disables redirects for auth-bearing requests and adds CarriesAuth helper.
MCPForUnity/Editor/Services/AssetGen/AssetGenModelCatalog.cs.meta Adds Unity meta for new model catalog.
MCPForUnity/Editor/Services/AssetGen/AssetGenModelCatalog.cs Adds curated model catalog + shared model resolution logic.
MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs Adds audio job flow and per-kind result extension allowlists enforced at write/import boundary.
MCPForUnity/Editor/Helpers/AssetGenPrefs.cs Adds audio provider pref and per-(kind, provider) model selection prefs.
MCPForUnity/Editor/Constants/EditorPrefKeys.cs Adds pref keys for audio provider and selected-model prefix.
Files not reviewed (4)
  • MCPForUnity/Editor/Services/AssetGen/AssetGenModelCatalog.cs.meta: Generated file
  • MCPForUnity/Editor/Services/AssetGen/Import/AudioImportPipeline.cs.meta: Generated file
  • MCPForUnity/Editor/Services/AssetGen/Providers/FalAudioAdapter.cs.meta: Generated file
  • MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs.meta: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 14 to 18
<uie:ToolbarToggle name="clients-tab" text="Connect" value="true" />
<uie:ToolbarToggle name="tools-tab" text="Tools" />
<uie:ToolbarToggle name="resources-tab" text="Resources" />
<uie:ToolbarToggle name="assetgen-tab" text="Asset Gen" />
<uie:ToolbarToggle name="assetgen-tab" text="Generative" />
<uie:ToolbarToggle name="deps-tab" text="Deps" />
Comment on lines +77 to +83
float dur = req.Duration > 0f ? req.Duration : entry.DefaultDurationSeconds;
float floor = Math.Max(1f, entry.MinDurationSeconds);
dur = Math.Min(Math.Max(dur, floor), entry.MaxDurationSeconds);
int seconds = (int)Math.Round(dur);
if (seconds < 1) seconds = 1;
body[entry.DurationField] = seconds;
}
Comment on lines +465 to +473
private static HashSet<string> AllowedExtensionsFor(string kind)
{
switch ((kind ?? string.Empty).ToLowerInvariant())
{
case "audio": return AudioAllowedExtensions;
case "image": return ImageAllowedExtensions;
default: return ModelAllowedExtensions; // model + marketplace
}
}

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs (1)

87-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared NormalizeOutputFolder and Cancel to reduce duplication.

NormalizeOutputFolder (lines 87-95) and Cancel (lines 122-129) are verbatim copies of the same methods in GenerateImage.cs. Status and ListProviders are also near-identical with only label differences. If a bug fix or validation change is applied to one copy, the others can silently diverge.

Consider extracting NormalizeOutputFolder into AssetGenPaths (it already depends on TryGetAssetsFolder) and Cancel into a shared AssetGenToolHelpers utility. The Status and ListProviders methods could accept a kind/label parameter to share a single implementation.

♻️ Proposed extraction for NormalizeOutputFolder
 // In AssetGenPaths.cs:
+public static bool NormalizeOutputFolder(string outputFolder, out string normalized, out string error)
+{
+    normalized = outputFolder;
+    error = null;
+    if (string.IsNullOrWhiteSpace(outputFolder)) return true;
+    if (TryGetAssetsFolder(outputFolder, out normalized)) return true;
+    error = "'output_folder' must resolve under the project's Assets folder.";
+    return false;
+}
 // In GenerateAudio.cs and GenerateImage.cs, replace:
-private static bool NormalizeOutputFolder(string outputFolder, out string normalized, out string error)
-{
-    normalized = outputFolder;
-    error = null;
-    if (string.IsNullOrWhiteSpace(outputFolder)) return true;
-    if (AssetGenPaths.TryGetAssetsFolder(outputFolder, out normalized)) return true;
-    error = "'output_folder' must resolve under the project's Assets folder.";
-    return false;
-}
+// (removed — call AssetGenPaths.NormalizeOutputFolder directly)

Also applies to: 122-129

🤖 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/Tools/AssetGen/GenerateAudio.cs` around lines 87 - 95,
Extract the duplicated NormalizeOutputFolder logic from GenerateAudio and
GenerateImage into AssetGenPaths, reusing TryGetAssetsFolder and preserving its
current outputs and validation behavior. Move the shared Cancel implementation
from both tools into AssetGenToolHelpers and update callers to use it.
Consolidate the near-identical Status and ListProviders implementations behind
shared helpers that accept the tool kind or label required for their messages.
🤖 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.

Nitpick comments:
In `@MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs`:
- Around line 87-95: Extract the duplicated NormalizeOutputFolder logic from
GenerateAudio and GenerateImage into AssetGenPaths, reusing TryGetAssetsFolder
and preserving its current outputs and validation behavior. Move the shared
Cancel implementation from both tools into AssetGenToolHelpers and update
callers to use it. Consolidate the near-identical Status and ListProviders
implementations behind shared helpers that accept the tool kind or label
required for their messages.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6619c51e-1e4c-4dd8-bc95-a07f1e8ca539

📥 Commits

Reviewing files that changed from the base of the PR and between dcf74e1 and ea3ce5a.

📒 Files selected for processing (55)
  • MCPForUnity/Editor/Constants/EditorPrefKeys.cs
  • MCPForUnity/Editor/Helpers/AssetGenPrefs.cs
  • MCPForUnity/Editor/Services/AssetGen/AssetGenJobManager.cs
  • MCPForUnity/Editor/Services/AssetGen/AssetGenModelCatalog.cs
  • MCPForUnity/Editor/Services/AssetGen/AssetGenModelCatalog.cs.meta
  • MCPForUnity/Editor/Services/AssetGen/Http/UnityWebRequestTransport.cs
  • MCPForUnity/Editor/Services/AssetGen/Import/AudioImportPipeline.cs
  • MCPForUnity/Editor/Services/AssetGen/Import/AudioImportPipeline.cs.meta
  • MCPForUnity/Editor/Services/AssetGen/Import/ImageImportPipeline.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/AssetGenProviders.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/FalAdapter.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/FalAudioAdapter.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/FalAudioAdapter.cs.meta
  • MCPForUnity/Editor/Services/AssetGen/Providers/IProviderAdapters.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/MeshyAdapter.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/OpenRouterAdapter.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/ProviderHttp.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/ProviderModels.cs
  • MCPForUnity/Editor/Services/AssetGen/Providers/TripoAdapter.cs
  • MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs
  • MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs.meta
  • MCPForUnity/Editor/Tools/AssetGen/GenerateImage.cs
  • MCPForUnity/Editor/Tools/AssetGen/GenerateModel.cs
  • MCPForUnity/Editor/Windows/Components/AssetGen/McpAssetGenSection.cs
  • MCPForUnity/Editor/Windows/Components/AssetGen/McpAssetGenSection.uxml
  • MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs
  • MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml
  • Server/src/cli/commands/asset_gen.py
  • Server/src/services/registry/tool_registry.py
  • Server/src/services/tools/generate_audio.py
  • Server/src/services/tools/generate_model.py
  • Server/tests/test_asset_gen_audio.py
  • Server/tests/test_asset_gen_model.py
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenJobManagerTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenModelCatalogTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenModelCatalogTests.cs.meta
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenPrefsTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AssetGenProvidersTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AudioImportPipelineTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/AudioImportPipelineTests.cs.meta
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/FalAdapterTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/FalAudioAdapterTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/FalAudioAdapterTests.cs.meta
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateAudioTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateAudioTests.cs.meta
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/GenerateImageTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/MeshyAdapterTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/TripoAdapterTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/UnityWebRequestTransportTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/AssetGen/UnityWebRequestTransportTests.cs.meta
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/Windows/Characterization/Windows_Characterization.cs
  • website/docs/reference/tools/asset_gen/generate_audio.md
  • website/docs/reference/tools/asset_gen/generate_model.md
  • website/docs/reference/tools/asset_gen/index.md
  • website/docs/reference/tools/index.md

…allowlist

- FalAudioAdapter.BuildBody: floor (not Math.Round) the clamped duration so
  it never exceeds the requested value, then enforce >= 1 (banker's rounding
  could round 2.5 -> 2 and could exceed the request).
- AssetGenJobManager.AllowedExtensionsFor: fail closed — an unexpected/unknown
  job kind now allows nothing instead of falling through to the model
  allowlist, so the RCE boundary never opens by default. Explicit cases for
  model + marketplace.
- Tests: pin floor semantics (10.9 -> 10) and the fail-closed behavior
  (unknown/null/empty kind -> not allowed).

Verified: EditMode 1167 tests / 0 failures + in-editor smoke (BuildBody(10.9)
-> duration=10; IsAllowed("bogus","glb")=false).

Claude-Session: https://claude.ai/code/session_015KYy51gwBuhDuLZXXoqc98
@Scriptwonder

Copy link
Copy Markdown
Collaborator Author

Thanks for the reviews 🙏 — addressed in 26f8a10:

Copilot

  • FalAudioAdapter.BuildBody — floor, don't round. Switched Math.Round → floor + enforce >= 1, so a clamped duration never exceeds the request and banker's rounding can't surprise. Regression test pins 10.9 → 10.
  • AllowedExtensionsFor fails closed. An unexpected/unknown job kind now returns an empty allowlist (explicit model/marketplace cases); the RCE boundary no longer opens by default. Test covers unknown/null/empty kind → rejected.
  • ⏸️ UI naming (.uxml tab "Generative" vs the "Asset Gen" group label). These are distinct surfaces — this PR fixes the tool-group display label; the tab was intentionally renamed to "Generative" in an earlier commit. Unifying the broader naming is a UX call I'd rather leave to the maintainer than change here.

CodeRabbit

  • ⏸️ Extract NormalizeOutputFolder / Cancel / Status / ListProviders. Valid maintainability point (3× duplication). Deferring to a focused follow-up to keep this PR scoped to the audio feature + security/correctness hardening — the tool-shell unification is already on the follow-up list.

Verified after the fixes: EditMode 1167 tests / 0 failures + in-editor smoke (BuildBody(10.9) → duration=10, IsAllowed("bogus","glb") = false).

status / cancel / list_providers were near-identical across GenerateAudio,
GenerateImage and GenerateModel (differing only by a kind label + poll
interval), and NormalizeOutputFolder was a verbatim triple copy.

- New AssetGenToolHelpers.{Status,Cancel,ListProviders} — the three tools now
  delegate, passing their kind label / poll interval.
- NormalizeOutputFolder moved to AssetGenPaths (it already lived on
  TryGetAssetsFolder).
- Behaviour preserved (image `remove_background` arm kept; provider-list
  filtering unchanged). Net -120 lines of duplication.

Verified: EditMode 1167 tests / 0 failures + in-editor smoke (audio/image/model
list_providers, status/cancel error paths, image remove_background all intact).

Claude-Session: https://claude.ai/code/session_015KYy51gwBuhDuLZXXoqc98
@Scriptwonder

Copy link
Copy Markdown
Collaborator Author

@coderabbitai Addressed the tool-shell duplication in 36cbe62:

  • New AssetGenToolHelpers.{Status, Cancel, ListProviders} — the three generate tools now delegate, passing only their kind label + poll interval.
  • NormalizeOutputFolder moved to AssetGenPaths (it already sat next to TryGetAssetsFolder).
  • Behaviour preserved: the image-only remove_background arm is kept and provider-list filtering is unchanged. Net −120 lines of duplication.

Verified: EditMode 1167 / 0 failures + in-editor smoke (audio/image/model list_providers, status/cancel error paths, and image remove_background all intact).

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd "AssetGenToolHelpers" 
fd "AssetGenPaths"
echo "---"
rg -n "NormalizeOutputFolder|class AssetGenToolHelpers|AssetGenToolHelpers\." MCPForUnity/Editor -g '*.cs' | head -80

Length of output: 2565


🏁 Script executed:

#!/bin/bash
sed -n '1,60p' MCPForUnity/Editor/Tools/AssetGen/AssetGenToolHelpers.cs
echo "---GenerateImage remove_background---"
rg -n "remove_background" MCPForUnity/Editor -g '*.cs'

Length of output: 3929


This confirms the refactor matches what was described — AssetGenToolHelpers.{Status, Cancel, ListProviders} now provide the shared logic, all three generate tools (GenerateAudio, GenerateImage, GenerateModel) delegate to it with their kind label and poll interval, NormalizeOutputFolder lives in AssetGenPaths alongside TryGetAssetsFolder, and the image-only remove_background arm is preserved intact in GenerateImage.cs. Nicely scoped extraction — thanks for following up on this.

🐇✨

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/Helpers/AssetGenPaths.cs`:
- Around line 89-93: Update NormalizeOutputFolder so whitespace-only
outputFolder values set normalized to null before returning true, while
preserving the existing behavior for non-whitespace paths and invalid inputs.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: e53e0081-0037-4079-86eb-c8ba9f9a7f69

📥 Commits

Reviewing files that changed from the base of the PR and between 26f8a10 and 36cbe62.

📒 Files selected for processing (6)
  • MCPForUnity/Editor/Helpers/AssetGenPaths.cs
  • MCPForUnity/Editor/Tools/AssetGen/AssetGenToolHelpers.cs
  • MCPForUnity/Editor/Tools/AssetGen/AssetGenToolHelpers.cs.meta
  • MCPForUnity/Editor/Tools/AssetGen/GenerateAudio.cs
  • MCPForUnity/Editor/Tools/AssetGen/GenerateImage.cs
  • MCPForUnity/Editor/Tools/AssetGen/GenerateModel.cs

Comment on lines +89 to +93
public static bool NormalizeOutputFolder(string outputFolder, out string normalized, out string error)
{
normalized = outputFolder;
error = null;
if (string.IsNullOrWhiteSpace(outputFolder)) return true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize whitespace-only outputFolder to null.

When outputFolder is whitespace-only (e.g. " "), normalized is set to the raw whitespace string and the method returns true. Downstream consumers receive a whitespace-only path that may bypass IsNullOrEmpty checks. Normalizing to null (consistent with AssetGenModelCatalog.ResolveModel which returns null for whitespace) makes the contract clearer: whitespace means "no folder specified."

🛡️ Proposed fix
         public static bool NormalizeOutputFolder(string outputFolder, out string normalized, out string error)
         {
-            normalized = outputFolder;
+            normalized = null;
             error = null;
-            if (string.IsNullOrWhiteSpace(outputFolder)) return true;
+            if (string.IsNullOrWhiteSpace(outputFolder)) return true;
             if (TryGetAssetsFolder(outputFolder, out normalized)) return true;
📝 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
public static bool NormalizeOutputFolder(string outputFolder, out string normalized, out string error)
{
normalized = outputFolder;
error = null;
if (string.IsNullOrWhiteSpace(outputFolder)) return true;
public static bool NormalizeOutputFolder(string outputFolder, out string normalized, out string error)
{
normalized = null;
error = null;
if (string.IsNullOrWhiteSpace(outputFolder)) return true;
🤖 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/Helpers/AssetGenPaths.cs` around lines 89 - 93, Update
NormalizeOutputFolder so whitespace-only outputFolder values set normalized to
null before returning true, while preserving the existing behavior for
non-whitespace paths and invalid inputs.

@Scriptwonder Scriptwonder merged commit 3db6b26 into CoplayDev:beta Jul 13, 2026
8 checks passed
@Scriptwonder Scriptwonder deleted the feat/assetgen-audio-model-panel branch July 13, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants