Update - #45
Conversation
* Add setup instructions to release workflow for voicevox-cli * Enhance instruction loading with XDG Base Directory support * Refactor instruction loading methods for clarity and consistency
Refactor configuration handling and remove unused setup code
Bumps [cachix/install-nix-action](https://github.com/cachix/install-nix-action) from 31.5.2 to 31.6.0. - [Release notes](https://github.com/cachix/install-nix-action/releases) - [Changelog](https://github.com/cachix/install-nix-action/blob/master/RELEASE.md) - [Commits](cachix/install-nix-action@fc6e360...56a7bb7) --- updated-dependencies: - dependency-name: cachix/install-nix-action dependency-version: 31.6.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.69 to 2.0.15. - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@1.0.69...2.0.15) --- updated-dependencies: - dependency-name: thiserror dependency-version: 2.0.15 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…ix/install-nix-action-31.6.0 ci(deps): bump cachix/install-nix-action from 31.5.2 to 31.6.0
deps(deps): bump thiserror from 1.0.69 to 2.0.15
…dates Bumps the patch-updates group with 4 updates in the / directory: [thiserror](https://github.com/dtolnay/thiserror), [clap](https://github.com/clap-rs/clap), [serde_json](https://github.com/serde-rs/json) and [mimalloc](https://github.com/purpleprotocol/mimalloc_rust). Updates `thiserror` from 2.0.15 to 2.0.16 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@2.0.15...2.0.16) Updates `clap` from 4.5.45 to 4.5.46 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.5.45...clap_complete-v4.5.46) Updates `serde_json` from 1.0.142 to 1.0.143 - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](serde-rs/json@v1.0.142...v1.0.143) Updates `mimalloc` from 0.1.47 to 0.1.48 - [Release notes](https://github.com/purpleprotocol/mimalloc_rust/releases) - [Commits](purpleprotocol/mimalloc_rust@v0.1.47...v0.1.48) --- updated-dependencies: - dependency-name: thiserror dependency-version: 2.0.16 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: clap dependency-version: 4.5.46 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: serde_json dependency-version: 1.0.143 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: mimalloc dependency-version: 0.1.48 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates ... Signed-off-by: dependabot[bot] <support@github.com>
…-c8414e3aa9 deps(deps): bump the patch-updates group across 1 directory with 4 updates
* Update README with improved setup instructions and prerequisites * Update instruction file references to VOICEVOX.md * Enhance tool descriptions for text_to_speech and list_voice_styles * Refactor VOICEVOX documentation and tool descriptions for clarity * Fix formatting issue in VOICEVOX.md by adding newline at EOF
…n permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…permissions Potential fix for code scanning alert no. 1: Workflow does not contain permissions
…P-execution-options
…sing-the-MCP-execution-options Refactor Serena MCP configuration and update dependencies
* Implement active request management and cancellation for MCP tools - Added `ActiveRequests` struct to manage active MCP requests and their cancellation tokens. - Implemented methods for registering, cancelling, and completing requests. - Enhanced `spawn_execution` to support cancellation during tool execution. - Refactored `run_mcp_server` to utilize `ActiveRequests` for handling incoming requests. - Updated tool execution logic in `tools.rs` to support cancellation for `text_to_speech`. - Removed unused `types.rs` file as its contents were integrated into other modules. * Add response channel to ActiveRequests for async tool execution * Enhance cancellation handling and debugging in MCP tools * Implement cancellation of active requests on client disconnect and shutdown * Refactor error handling and logging in MCP server and tools * Improve cancellable audio playback handling in synthesis function * Add cancellation request handling and tests for MCP protocol * Add tempfile dependency and improve audio playback cancellation * Refactor audio playback cancellation handling in tools.rs * Fix audio playback cancellation by retaining stream guard * Use existing runtime handle for tool request execution
Bumps the patch-updates group with 6 updates: | Package | From | To | | --- | --- | --- | | [anyhow](https://github.com/dtolnay/anyhow) | `1.0.99` | `1.0.100` | | [thiserror](https://github.com/dtolnay/thiserror) | `2.0.16` | `2.0.17` | | [clap](https://github.com/clap-rs/clap) | `4.5.46` | `4.5.48` | | [serde](https://github.com/serde-rs/serde) | `1.0.219` | `1.0.228` | | [serde_json](https://github.com/serde-rs/json) | `1.0.143` | `1.0.145` | | [libc](https://github.com/rust-lang/libc) | `0.2.175` | `0.2.176` | Updates `anyhow` from 1.0.99 to 1.0.100 - [Release notes](https://github.com/dtolnay/anyhow/releases) - [Commits](dtolnay/anyhow@1.0.99...1.0.100) Updates `thiserror` from 2.0.16 to 2.0.17 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@2.0.16...2.0.17) Updates `clap` from 4.5.46 to 4.5.48 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.5.46...clap_complete-v4.5.48) Updates `serde` from 1.0.219 to 1.0.228 - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.219...v1.0.228) Updates `serde_json` from 1.0.143 to 1.0.145 - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](serde-rs/json@v1.0.143...v1.0.145) Updates `libc` from 0.2.175 to 0.2.176 - [Release notes](https://github.com/rust-lang/libc/releases) - [Changelog](https://github.com/rust-lang/libc/blob/0.2.176/CHANGELOG.md) - [Commits](rust-lang/libc@0.2.175...0.2.176) --- updated-dependencies: - dependency-name: anyhow dependency-version: 1.0.100 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: thiserror dependency-version: 2.0.17 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: clap dependency-version: 4.5.48 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: serde dependency-version: 1.0.228 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: serde_json dependency-version: 1.0.145 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates - dependency-name: libc dependency-version: 0.2.176 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch-updates ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps the minor-updates group with 1 update: [tempfile](https://github.com/Stebalien/tempfile). Updates `tempfile` from 3.20.0 to 3.23.0 - [Changelog](https://github.com/Stebalien/tempfile/blob/master/CHANGELOG.md) - [Commits](Stebalien/tempfile@v3.20.0...v3.23.0) --- updated-dependencies: - dependency-name: tempfile dependency-version: 3.23.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-updates ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [cachix/install-nix-action](https://github.com/cachix/install-nix-action) from 31.6.0 to 31.7.0. - [Release notes](https://github.com/cachix/install-nix-action/releases) - [Changelog](https://github.com/cachix/install-nix-action/blob/master/RELEASE.md) - [Commits](cachix/install-nix-action@56a7bb7...9280e7a) --- updated-dependencies: - dependency-name: cachix/install-nix-action dependency-version: 31.7.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [softprops/action-gh-release](https://github.com/softprops/action-gh-release) from 2.3.2 to 2.3.3. - [Release notes](https://github.com/softprops/action-gh-release/releases) - [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md) - [Commits](softprops/action-gh-release@72f2c25...6cbd405) --- updated-dependencies: - dependency-name: softprops/action-gh-release dependency-version: 2.3.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…-07f42315d2 deps(deps): bump the patch-updates group with 6 updates
…-2d8dee542f deps(deps): bump tempfile from 3.20.0 to 3.23.0 in the minor-updates group
…nix-action-31.7.0
…ix/install-nix-action-31.7.0 ci(deps): bump cachix/install-nix-action from 31.6.0 to 31.7.0
…n-gh-release-2.3.3
…props/action-gh-release-2.3.3 ci(deps): bump softprops/action-gh-release from 2.3.2 to 2.3.3
Summary of ChangesHello @usabarashi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the robustness, modularity, and user-friendliness of the VOICEVOX CLI, particularly its Model Context Protocol (MCP) server. Key improvements include a complete refactoring of the MCP server's internal architecture to support cancellable tool execution, adopting XDG Base Directory standards for configuration, and streamlining the development and CI environments. These changes aim to provide a more stable and configurable experience for users and developers alike, while also updating underlying dependencies for better performance and security. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Code refactored, clean and new, Modules shine, a clearer view. No more old paths, now XDG, Instructions flow, for all to see. Dependencies, updated, true. Footnotes
|
There was a problem hiding this comment.
Pull Request Overview
Refactors MCP server architecture with protocol separation, introduces cancellation support for tool executions, simplifies configuration and setup logic, and removes numerous logging/output and legacy setup/config code. Key changes:
- MCP subsystem refactored (new protocol, requests, tools) with async execution and cancellation; JSON-RPC / instruction loading logic reworked (XDG-based).
- Removed configuration load/save and setup/model bootstrap code; replaced Config::load usage with Config::default; reduced logging and error reporting across multiple modules.
- Updated build/packaging (flake.nix simplification, CI scripts), added temp file audio handling and cancellation-capable playback.
Reviewed Changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/voice.rs | Suppressed model load/unload/logging errors during mapping to simplify output. |
| src/synthesis/streaming.rs | Replaced persisted config loading with default config instantiation. |
| src/setup.rs | Removed first-run model setup helper. |
| src/paths.rs | Removed models dir helper and silenced unexpected filename warning. |
| src/mcp/types.rs | Removed old MCP type definitions (replaced by protocol.rs). |
| src/mcp/tools.rs | Added full MCP tool framework with cancellation, playback variants, validation, tests. |
| src/mcp/server.rs | Rewrote server loop with request routing, response channel, cancellation integration. |
| src/mcp/requests.rs | New active request tracking & async execution management. |
| src/mcp/protocol.rs | New MCP protocol definitions, instruction loading (XDG), request/notification handling. |
| src/mcp/mod.rs | Updated module exports to new structure. |
| src/lib.rs | Removed setup module export. |
| src/daemon/server.rs | Removed verbose logging; simplified model load/unload and error handling. |
| src/daemon/process.rs | Suppressed warning logging on process discovery failure. |
| src/config.rs | Removed file-based config load/save; left only struct defaults. |
| src/client/audio.rs | Added robust temp WAV file creation; refactored playback logic. |
| scripts/ci.sh | Added helper function; streamlined artifact and tool invocation. |
| flake.nix | Simplified flake, removed auxiliary tooling scripts, adjusted inputs and dev shell. |
| docs/mcp-usage.md | Updated instruction loading documentation to XDG-based VOICEVOX.md. |
| VOICEVOX.md | New instruction file replacing INSTRUCTIONS.md. |
| README.md | Updated installation instructions for non-Nix distribution. |
| INSTRUCTIONS.md | Removed legacy instruction file. |
| Cargo.toml | Dependency version adjustments; removed unused crates. |
| CLAUDE.md | Updated instruction loading priority documentation. |
| .mcp.json | Updated Serena MCP server invocation method/environment. |
| .github/workflows/* | Bumped action versions; minor permissions addition. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| inputs = { | ||
| nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; | ||
| nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable"; |
There was a problem hiding this comment.
The input URL uses 'nixpkgs/nixpkgs-unstable' which is not a standard ref; the usual channel is 'nixos-unstable' (i.e. github:NixOS/nixpkgs/nixos-unstable) or a rev/ref pair. This will likely fail to resolve; change to 'github:NixOS/nixpkgs/nixos-unstable' unless you intended a different ref.
| nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable"; | |
| nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; |
| let runtime_handle = tokio::runtime::Handle::current(); | ||
|
|
||
| tokio::task::spawn_blocking(move || { | ||
| // Use current runtime handle instead of creating a new one | ||
| runtime_handle.block_on(async move { | ||
| let result = | ||
| tools::execute_tool_request(&tool_name, arguments, Some(abort_rx)).await; | ||
|
|
||
| // Clean up the request from active list | ||
| active_requests.complete(&request_id).await; | ||
|
|
||
| // Send response | ||
| let response = match result { | ||
| Ok(tool_result) => match serde_json::to_value(tool_result) { | ||
| Ok(value) => JsonRpcResponse::success(id, value), | ||
| Err(_) => JsonRpcResponse::error( | ||
| id, | ||
| INTERNAL_ERROR, | ||
| "Failed to serialize response".to_string(), | ||
| ), | ||
| }, | ||
| Err(e) => { | ||
| let error_result = ToolCallResult { | ||
| content: vec![ToolContent { | ||
| content_type: "text".to_string(), | ||
| text: format!("Tool execution error: {e}"), | ||
| }], | ||
| is_error: Some(true), | ||
| }; | ||
| match serde_json::to_value(error_result) { | ||
| Ok(value) => JsonRpcResponse::success(id, value), | ||
| Err(_) => JsonRpcResponse::error( | ||
| id, | ||
| INTERNAL_ERROR, | ||
| "Failed to serialize error response".to_string(), | ||
| ), | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| // Send response via channel | ||
| let _ = active_requests.response_sender.send(response); | ||
| }) |
There was a problem hiding this comment.
Using spawn_blocking + runtime_handle.block_on to run async code introduces unnecessary indirection and can reduce scalability. Prefer spawning the async task directly with tokio::spawn (move async { ... }) and await the tool execution, unless the code inside is truly CPU-bound/blocking.
| let runtime_handle = tokio::runtime::Handle::current(); | |
| tokio::task::spawn_blocking(move || { | |
| // Use current runtime handle instead of creating a new one | |
| runtime_handle.block_on(async move { | |
| let result = | |
| tools::execute_tool_request(&tool_name, arguments, Some(abort_rx)).await; | |
| // Clean up the request from active list | |
| active_requests.complete(&request_id).await; | |
| // Send response | |
| let response = match result { | |
| Ok(tool_result) => match serde_json::to_value(tool_result) { | |
| Ok(value) => JsonRpcResponse::success(id, value), | |
| Err(_) => JsonRpcResponse::error( | |
| id, | |
| INTERNAL_ERROR, | |
| "Failed to serialize response".to_string(), | |
| ), | |
| }, | |
| Err(e) => { | |
| let error_result = ToolCallResult { | |
| content: vec![ToolContent { | |
| content_type: "text".to_string(), | |
| text: format!("Tool execution error: {e}"), | |
| }], | |
| is_error: Some(true), | |
| }; | |
| match serde_json::to_value(error_result) { | |
| Ok(value) => JsonRpcResponse::success(id, value), | |
| Err(_) => JsonRpcResponse::error( | |
| id, | |
| INTERNAL_ERROR, | |
| "Failed to serialize error response".to_string(), | |
| ), | |
| } | |
| } | |
| }; | |
| // Send response via channel | |
| let _ = active_requests.response_sender.send(response); | |
| }) | |
| tokio::spawn(async move { | |
| let result = | |
| tools::execute_tool_request(&tool_name, arguments, Some(abort_rx)).await; | |
| // Clean up the request from active list | |
| active_requests.complete(&request_id).await; | |
| // Send response | |
| let response = match result { | |
| Ok(tool_result) => match serde_json::to_value(tool_result) { | |
| Ok(value) => JsonRpcResponse::success(id, value), | |
| Err(_) => JsonRpcResponse::error( | |
| id, | |
| INTERNAL_ERROR, | |
| "Failed to serialize response".to_string(), | |
| ), | |
| }, | |
| Err(e) => { | |
| let error_result = ToolCallResult { | |
| content: vec![ToolContent { | |
| content_type: "text".to_string(), | |
| text: format!("Tool execution error: {e}"), | |
| }], | |
| is_error: Some(true), | |
| }; | |
| match serde_json::to_value(error_result) { | |
| Ok(value) => JsonRpcResponse::success(id, value), | |
| Err(_) => JsonRpcResponse::error( | |
| id, | |
| INTERNAL_ERROR, | |
| "Failed to serialize error response".to_string(), | |
| ), | |
| } | |
| } | |
| }; | |
| // Send response via channel | |
| let _ = active_requests.response_sender.send(response); |
| if core.load_specific_model(&model_id.to_string()).is_err() { | ||
| continue; |
There was a problem hiding this comment.
Previously this path logged model load failures; now errors are silently ignored, making it harder to diagnose missing styles/models. Consider at least emitting a trace/debug log or aggregating failures for later reporting.
| let _expected_patterns = if cfg!(target_os = "macos") { | ||
| "libonnxruntime.dylib or libvoicevox_onnxruntime.*.dylib" | ||
| } else if cfg!(target_os = "linux") { | ||
| "libonnxruntime.so or libvoicevox_onnxruntime.*.so" | ||
| } else { | ||
| "onnxruntime.dll, libonnxruntime.dll, or libvoicevox_onnxruntime.*.dll" | ||
| }; |
There was a problem hiding this comment.
The assigned variable '_expected_patterns' is never used; this entire block becomes dead code after removing the warning log. Remove it (or reintroduce a log) to avoid misleading future readers.
| let _expected_patterns = if cfg!(target_os = "macos") { | |
| "libonnxruntime.dylib or libvoicevox_onnxruntime.*.dylib" | |
| } else if cfg!(target_os = "linux") { | |
| "libonnxruntime.so or libvoicevox_onnxruntime.*.so" | |
| } else { | |
| "onnxruntime.dll, libonnxruntime.dll, or libvoicevox_onnxruntime.*.dll" | |
| }; | |
| // Invalid ORT_DYLIB_PATH filename; expected platform-specific patterns. |
| pub async fn new() -> Result<Self> { | ||
| let daemon_client = DaemonClient::connect_with_retry().await?; | ||
| let config = Config::load().unwrap_or_default(); | ||
| let config = Config::default(); |
There was a problem hiding this comment.
Replacing Config::load().unwrap_or_default() with Config::default() removes any persisted/user configuration capability that may have existed; this is a silent behavior change for callers expecting overrides. If intentional, document the deprecation or keep a fall-back attempt to load while tolerating failure.
| let config = Config::default(); | |
| let config = Config::load().unwrap_or_default(); |
| Ok((req, _)) => req, | ||
| Err(e) => { | ||
| println!("Failed to deserialize request: {e}"); | ||
| Err(_) => { |
There was a problem hiding this comment.
Deserialization errors now cause a silent break without logging, making it difficult to diagnose protocol incompatibilities or corrupted frames. Consider logging at least a concise warning before breaking.
| Err(_) => { | |
| Err(err) => { | |
| eprintln!("Warning: Failed to deserialize DaemonRequest: {}", err); |
| eprintln!("Warning: Failed to find daemon processes: {}", e); | ||
| 0 | ||
| } | ||
| Err(_) => 0, |
There was a problem hiding this comment.
Swallowing the underlying error when finding daemon processes removes potentially useful diagnostic information (e.g., pgrep not present). Retain minimal logging or map specific errors if noisy output was the concern.
| Err(_) => 0, | |
| Err(e) => { | |
| eprintln!("Failed to find daemon processes: {}", e); | |
| 0 | |
| }, |
No description provided.