feat(tools): add MiniMax voice cloning - #305
Open
octo-patch wants to merge 2 commits into
Open
Conversation
12 tasks
Contributor
|
Nice addition. One thing worth settling before this lands:
The tool is declared configured, never registered, and nothing says why. Either rule is defensible; they just need to be the same one. If Heads-up on the interaction with #312, which adds a table describing exactly The fix is one entry in |
Author
|
Thanks for the review — good catch, and I settled it toward the rule that already exists. On #312: |
Handsome-wzw
added a commit
that referenced
this pull request
Aug 25, 2026
…ities visible (#312) ## Summary A WeChat user asked for a recommendation and the bot answered "the web search tool is not configured here". That sentence was not the model's own phrasing -- it was the tool's error text, relayed outward. `web_search` needs a Serper key and that deployment has never had one, but the tool was registered unconditionally, so on every search-shaped question the model saw it, reached for it, and passed the setup error on to whoever was in the chat. Withholding the tool fixes that and creates a second problem: an unregistered tool is invisible. Nothing in a running Raven then says the capability exists at all -- the model is never offered it, no document lists it, and `raven doctor` reports on providers and memory but has never mentioned tools. The only way to learn that web search is one account and one edit away was to read the source. This does both halves, because either alone leaves a real gap: the deployer is told what this install can and cannot do, and the model is still not offered a tool it cannot run. ### Withholding the unusable tool `web_search` is gated on a resolved key, in the main loop and in the sub-agent surface (`subagent/manager.py`) -- a sub-agent that reaches for a search it cannot run reports the failure to its caller, and that text lands in the parent turn, the same leak one level down. The gate asks the tool, not the config. `WebSearchTool.api_key` resolves at call time from the constructor value *or* `SERPER_API_KEY`, so reading `tools.web.search.apiKey` alone would withdraw a working tool from any deploy that exports the variable and configures nothing. The tool's error message also hard-coded `~/.raven/config.json` while the gateway runs with `--config` elsewhere, so following it meant editing a file the process never reads. It names the path actually in force now. That text is reachable only if the key disappears after registration, which is exactly why it should be right: it is the message for the case the gate cannot cover. ### Three rules, and nowhere to read them Registration is decided per family, each a different shape: | family | rule | |---|---| | `web_search` | a resolved key, asked of the built tool | | `web_fetch` | nothing -- always registered, a key only improves extraction | | media x3 | an `api_key` *or* a `model`, either counting as configured | For media those are two questions, not one. A section naming only a model is registered, because a model alone counts as asking for the tool, and then every call returns a missing-key error. Whether a capability is *offered* and whether it *works* come apart there, and a report treating them as one fact ticks a capability that cannot run. Each rule is defensible where it sits. What is missing is anywhere to read them. Providers had the same sprawl once and answered it with `providers.auth`: a declarative table plus `credential_status` as the single authority, with an AST invariant enforcing that authority, because six surfaces had answered the same question six ways and each looked reasonable alone. Tools never got the equivalent. ### What this adds **Each tool answers for itself,** twice where the questions differ. `WebSearchTool.is_configured` reads the config value *or* `SERPER_API_KEY`, because those are two sources and only the tool consults both. The media base answers `is_configured` on a model *or* a key -- which is what stops an OpenRouter credential set for chat from silently switching on three tools that bill per call -- and answers `has_key` separately on the chain it actually resolves at call time: its own section, the borrowed provider key, then `OPENROUTER_API_KEY`. Both rules live with the credential they read. **`capabilities.py` describes the five** for a human deciding what to set up: what each does in one line, how much work it is (nothing / reuse a credential you already have / obtain an account), where the key goes, where to get one, and what it costs. It rules on nothing -- `is_configured` and `has_credential` both ask the tools -- so it cannot become a second opinion. **`raven doctor` grows a section**, listing every capability configured or not, ordered by how much the deployer has to do: ``` (markers render as a green check and a yellow bang; spelled [ok] and [!] here to keep this description ASCII, since it becomes the squash commit body) Tool capabilities web_fetch: [ok] Read a web page the agent already has the URL for image_generate: [ok] Generate an image (borrowed: providers.openrouter.apiKey) text_to_speech: [!] Generate speech from text no key resolves; calls will fail set: tools.media.speech.apiKey or env: OPENROUTER_API_KEY video_generate: - Generate a video switch on: tools.media.video.model key: reusing providers.openrouter.apiKey Billed per call; needs prepaid OpenRouter credit. web_search: - Search the web set: tools.web.search.apiKey or env: SERPER_API_KEY key from: https://serper.dev 2 capability(s) available but not set up; the agent is not offered them. ``` Three deliberate details. Naming the credential is load-bearing in both directions. A row that cannot distinguish a reused credential from a missing one sends someone to create an account they already have -- and a row that claims a reuse with nothing to reuse is worse, because acting on it means setting a model, getting a registered tool, and watching every call fail on a credential they were told they had. So the reuse line prints only when a key is genuinely there to pick up, and a capability already in that broken state says so outright instead of showing a satisfied tick. The credential is named at the path that holds it. For the media family `config_path` names the *model*, so reusing it as the key source pointed the deployer at a line with no credential in it. And each fact is on its own line rather than in a sentence, because the terminal wraps a long line mid-path and a config key broken across two rows cannot be copied, which is the only thing that row is for. Nothing here moves the exit code, including the warned row. An install without image generation is a choice, not a fault, and a doctor that fails on it teaches people to ignore doctor. The half-finished one is arguable -- the memory section does exit non-zero for a role the user configured that the server could not build -- but that failure is silent where it happens, recall just returning nothing, whereas this one returns an error string to the model on every call. Say the word and it becomes an exit code instead. ### Not in this change Registration still lives in `AgentLoop`. Having it read the table is the point of this shape and removes the last duplicate reader, but it edits `agent/loop/main.py`, which is under active change, and it is worth doing on its own once that settles. Until then the table is a description, and the tests below are what keep it honest. `deep_research` is deliberately absent: it is moving to the sub-agent surface and its tool is going away. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other Mixed on purpose: the first commit is a fix and the rest are the feature it made necessary. Splitting them would land a change that hides a capability without landing the one that makes it discoverable. ## Verification ``` make lint exit 0 make build exit 0 npm run test --prefix ui-tui 969 passed, 13 skipped uv run --all-extras pytest -q 6186 passed, 35 skipped, 1 failed uv run pytest tests/test_tool_capabilities.py \ tests/test_cli_doctor_commands.py \ tests/test_agent_loop_web_tools.py 64 passed uv run --extra dev ruff check raven tests scripts All checks passed uv run --extra dev ruff format --check 828 files already formatted npx commitlint --from origin/main --to HEAD exit 0 scripts/check_commit_messages.py origin/main..HEAD exit 0 ``` The one failure is `test_read_file_image.py::test_an_attachment_that_cannot_be_ read_costs_a_note_not_the_turn`, and it is not from this branch: it fails the same way on `main` at `1cb604a` with these commits absent. The case makes a file unreadable with `chmod 000`, which does not block a root user, so it fails for anyone running the suite as root and passes in CI. This branch does not touch that file or the code under it. The capability tests drive a real `AgentLoop` and compare what it registered against what the table predicts, rather than asserting the table against itself. The gated set is derived -- tools present once credentials are supplied, absent without -- so a sixth gated tool whose author forgets the table fails here rather than going unnoticed. That last claim was false when first written, and the last commit is what makes it true. The fixture listing the media tools by hand never switched a fourth one on, so a new one never joined the gated set and the assertion held over an already-incomplete table. #305, which adds a MiniMax voice-clone tool, is that case: merged against this branch the assertion passed. Read from `MediaGenConfig` instead, it fails and names the tool -- `gated but undeclared: ['voice_clone']`. Which means whichever of the two lands second turns this red, deliberately. The fix is one table entry, and it needs the rule that PR settles: `voice_clone` counts as configured on `api_key or api_base or model` in `effective_media_config` while registration still gates on `api_key or model`, so an `apiBase`-only install with no MiniMax key is configured by one rule and withheld by the other. That is the divergence this table exists to make visible, and it is worth resolving there rather than papering over here. Fifteen mutations, each caught: ``` the web_search gate removed from the main loop 3 failed the same gate removed from the sub-agent surface 1 failed media rule reads only the key 7 failed web_search rule reads only the config 1 failed a tool removed from the table 6 failed the doctor section not rendered 6 failed doctor lists only configured tools 4 failed unconfigured treated as a failure 17 failed the reuse line is printed unconditionally 1 failed the key source falls back to the model path 3 failed a keyless registered capability is not flagged 1 failed has_key always answers yes 1 failed has_credential collapses into is_configured 1 failed the credential chain drops OPENROUTER_API_KEY 3 failed the reuse check ignores the environment 3 failed ``` Four of those tests exist only because a mutation pass found the earlier versions insufficient, and the last rounds are why the shape changed. A media case that sets both a model and an OpenRouter key proves nothing about the "or model" half: the borrow fills the key in, so a rule reading only the key still answers correctly. The case that pins it has nothing to borrow. The sub-agent case is the same argument applied to the second call site: with the gate present only in the main loop, every test above still passes, because nothing was watching what the sub-agent surface registers. More usefully, mutating `has_key` to always answer yes changed no test at all in the first version, which said the ruling was not load-bearing: the doctor was inferring "no credential" from an *empty source string* rather than from the tool's answer, so a fourth credential source would have been reported as a missing one. `has_credential` is now its own fact, asked of the tools, and that mutation fails. Separately, autouse fixtures clear `SERPER_API_KEY` and `OPENROUTER_API_KEY`, without which several of these pass for the wrong reason on any machine where a developer exported one. Rendering was checked against a real config in six states -- nothing set, a provider key present, a model with nothing to borrow, a tool with its own key, a model plus a borrowable key, and an exported variable as the only source -- rather than only asserted on. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk One behaviour change: `web_search` is no longer offered to the model when no key resolves. That is the fix. A deployment that has a key, in config or in the environment, is unaffected; one that has none was getting an error string in place of an answer. The rest is additive. No other registration logic changed, so which tools an agent is offered is otherwise exactly what it was; `is_configured` moved the existing predicates onto the tools without altering them, and the mutation results above are what pins that. `has_key` and `_resolve_key` are new names for the chain `api_key` already resolved -- the property calls the extracted one, so callers see the same answers. `raven doctor` gains a section and no new exit code. Its zero-network guarantee holds: the table reads config and environment only. Two allowlist entries were added to `test_only_the_auth_module_decides_configuredness_from_a_key`, argued in place: `subagent/manager.py`, which asks the built tool whether a key resolved so an unusable search is withheld, and `capabilities.py`, which reads keys to *report* -- which source supplied one, and whether one is there to reuse. Both ruling halves are delegated to the tools, whose files were already listed. Rollback is a revert. Nothing is written and no configuration is read differently. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
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
2 participants
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: Add executable voice cloning for the configured MiniMax backend.
Summary
voice_clonetool that uploads MP3, M4A, or WAV source audio and creates a reusable voice.Type
Verification
uv run --frozen pytest -q -p no:everos_plugin tests/test_media_gen_voice_clone_minimax.py(4 passed)uv run --frozen pytest -q -p no:everos_plugin tests/test_config_raven_sections.py(20 passed)uv run --frozen ruff format --check raven/agent/tools/media_gen.py raven/agent/loop/main.py raven/config/schema.py tests/test_media_gen_voice_clone_minimax.pyuv run --frozen ruff check --ignore I001 raven/agent/tools/media_gen.py raven/agent/loop/main.py raven/config/schema.py tests/test_media_gen_voice_clone_minimax.pyuv run --frozen python -m py_compile raven/agent/tools/media_gen.py raven/agent/loop/main.py raven/config/schema.py tests/test_media_gen_voice_clone_minimax.pygit diff origin/main...HEAD --checkRelevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
The tool is opt-in and preserves existing media behavior. Rollback consists of removing the tool registration and its configuration field.
Related Issues
N/A