Skip to content

feat: bundle Claude Code agent skill at .claude/skills/omnivoice/ - #113

Merged
debpalash merged 4 commits into
debpalash:mainfrom
broomva:feat/claude-skill-bundle
May 29, 2026
Merged

feat: bundle Claude Code agent skill at .claude/skills/omnivoice/#113
debpalash merged 4 commits into
debpalash:mainfrom
broomva:feat/claude-skill-bundle

Conversation

@broomva

@broomva broomva commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

CLAUDE.md already invites contributions at .claude/skills/:

"No project skills found. Add skills to any of: .claude/skills/, .agents/skills/, .cursor/skills/, .github/skills/, or .codex/skills/ with a SKILL.md index file."

But the existing .gitignore blanket-ignored .claude/ (line 41), making the invited path un-trackable. This PR narrows the ignore so ad-hoc Claude state stays out while deliberate skill bundles are tracked:

-.claude/
+.claude/*
+!.claude/skills/
+!.claude/skills/**

What the skill provides

Once merged, anyone running npx skills add debpalash/OmniVoice-Studio (the Vercel-Labs skills CLI) gets immediate agent context on:

  • What the MCP server exposes — 5 tools (generate_speech, list_voices, list_personalities, list_languages, check_health) + 2 resources (voice://{id}, history://recent)
  • When to pick OmniVoice vs other engines — decision rule in body (multilingual / cloning / privacy → OmniVoice; one-off English on weak hardware → kokoro/Edge TTS; highest English polish → ElevenLabs)
  • How to wire the stdio MCP server into a client config (with sample ~/.claude.json snippet)
  • Backend lifecyclestart-backend.sh (idempotent uvicorn boot + 60 s health probe), check-health.sh, stop-backend.sh
  • Common failure modes + fixes — port collision, model download stall, missing HF_TOKEN, MPS fallback, voice-profile-not-found, FastMCP kwargs (latter cross-referenced to fix(mcp): drop unsupported FastMCP kwargs (mcp SDK >= 1.10) #112)

Layout

.claude/skills/omnivoice/
├── SKILL.md                     # YAML frontmatter + tool index + workflows + decision rule
├── references/
│   └── mcp-setup.md             # lifecycle, env vars, troubleshooting
└── scripts/
    ├── check-health.sh          # curl /health, exit 0/1
    ├── start-backend.sh         # idempotent uvicorn boot + 60s health probe;
    │                            # resolves repo root from script path (works from any CWD)
    └── stop-backend.sh          # graceful SIGTERM on bound PID

Conventions followed

Conforms to Anthropic skill-creator conventions:

  • Frontmatter description under 1024-char limit (validates clean)
  • Body under 500 lines (~120 LOC)
  • references/ for detail, scripts/ for deterministic ops
  • No README.md / CHANGELOG.md / setup-guide files inside the skill (per skill-creator's "what NOT to include")
  • Validates clean against skill-creator/scripts/quick_validate.py

Test plan

  • npx skills list discovers the bundled skill once the directory exists
  • quick_validate.py reports Skill is valid!
  • scripts/check-health.sh against a running backend prints health JSON, exits 0
  • scripts/start-backend.sh from any CWD resolves the repo root correctly via BASH_SOURCE traversal
  • End-to-end MCP workflows verified (depends on fix(mcp): drop unsupported FastMCP kwargs (mcp SDK >= 1.10) #112 to actually run the MCP server):
    • generate_speech English with demo voice (16 steps) → 4.2 s WAV
    • generate_speech voice design via instruct only (8 steps) → 6.3 s WAV
    • generate_speech Spanish with demo voice (16 steps) → 2.8 s WAV
  • uv run pytest backend/ -x -q — 45 passed (no changes to backend code in this PR)

Cross-platform note

Per the project's strict cross-platform-default rule, the three helper scripts use only POSIX bash + curl + lsof + kill. They work on macOS and Linux out of the box. Windows users wire MCP via the documented ~/.claude.json snippet and run the backend directly without the helpers — same path the desktop installer takes.

Dependency

Depends on #112 (FastMCP API fix). Without that fix, every MCP tool call fails with TypeError at server construction, which would make this skill broken-by-default for any user installing it.

The PR diff currently shows only the skill files (the FastMCP fix is on a separate branch). Once #112 merges, this branch can be rebased and the diff stays identical.

Summary by CodeRabbit

  • Documentation

    • Comprehensive OmniVoice docs added: local server setup, lifecycle and health checks, API/tool index, usage patterns (narration, WAV export, end-to-end voice cloning, instruct-based voice design), engine comparison guidance, troubleshooting, and links to API docs/Swagger. Notes that the video-dubbing pipeline is UI/REST-only (not exposed via MCP).
  • New Features

    • Utility scripts for backend health checks, start/stop lifecycle, and a macOS helper to record voice-reference clips.
  • Chores

    • Updated ignore rules to keep bundled skill docs while ignoring other local metadata.

Review Change Stack

broomva added 2 commits May 20, 2026 12:01
The MCP server passes `version=` and `description=` to FastMCP(), but
neither kwarg exists on mcp >= 1.10 — the protocol version is now
managed internally and `description` was renamed to `instructions`.

Symptom on a fresh install (uv sync && pip install 'mcp[cli]'):

    TypeError: FastMCP.__init__() got an unexpected keyword argument 'version'

Tested locally end-to-end:
- create_mcp_server() now constructs cleanly
- All 5 tools register and are listable via FastMCP.list_tools()
- generate_speech round-trip returns base64 WAV; ~24s server-side
  for 4.2s of audio at steps=16 on Apple Silicon MPS
- pytest backend/ -x -q: 45 passed
CLAUDE.md already invites contributions at .claude/skills/:

  "No project skills found. Add skills to any of: .claude/skills/,
   .agents/skills/, .cursor/skills/, .github/skills/, or .codex/skills/
   with a SKILL.md index file."

But the existing .gitignore blanket-ignored .claude/ (line 41), making
the invited path un-trackable. This commit narrows the ignore so ad-hoc
Claude state stays out while deliberate skill bundles are tracked:

    -.claude/
    +.claude/*
    +!.claude/skills/
    +!.claude/skills/**

Once merged, any compatible agent client running
`npx skills add debpalash/OmniVoice-Studio` gets immediate context on:

- What the MCP server exposes (5 tools + 2 resources)
- When to pick OmniVoice vs other engines
- How to wire the stdio MCP server into a client config
- Backend lifecycle: start / health / stop scripts
- Common failure modes + fixes (port collision, model download stall,
  missing HF_TOKEN, MPS fallback, voice-profile-not-found, etc.)

Conforms to Anthropic skill-creator conventions: frontmatter
description under 1024-char limit, body under 500 lines, references/
for detail, scripts/ for deterministic ops, no README/CHANGELOG
inside the skill, validates clean against quick_validate.py.

Verified locally that `npx skills list` discovers the bundled skill
automatically once cloned. End-to-end tested through MCP:
- generate_speech (English, demo voice, steps=16) -> 4.2 s WAV
- generate_speech (voice design via instruct only, steps=8) -> 6.3 s WAV
- generate_speech (Spanish, demo voice, steps=16) -> 2.8 s WAV

Depends on debpalash#112 (FastMCP API fix). Without it, every MCP tool call
fails with TypeError at server construction.
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds OmniVoice MCP skill documentation and a detailed MCP setup guide, three backend lifecycle scripts (health, start, stop), a macOS reference-recording helper, updates .gitignore to track skills, and adjusts the FastMCP constructor argument to use instructions=(...).

Changes

OmniVoice MCP Agent Skill Documentation and Backend Tooling

Layer / File(s) Summary
MCP documentation and gitignore
.claude/skills/omnivoice/SKILL.md, .claude/skills/omnivoice/references/mcp-setup.md, .claude/skills/omnivoice/references/engines-comparison.md, .gitignore
Adds the OmniVoice skill README, a detailed MCP setup reference, an engines comparison doc, and updates .gitignore to keep .claude/skills/ tracked.
Backend lifecycle scripts
.claude/skills/omnivoice/scripts/check-health.sh, .claude/skills/omnivoice/scripts/start-backend.sh, .claude/skills/omnivoice/scripts/stop-backend.sh
Adds a health probe script, an idempotent startup script that launches uvicorn and polls /health, and a shutdown script that SIGTERM-then-SIGKILLs listeners on port 3900 if needed.
Reference recording helper (macOS)
.claude/skills/omnivoice/scripts/record-reference.sh
Adds a macOS-only script to record a 24kHz mono reference clip, trim silence, verify levels/playback, and print a sample curl for creating a voice profile.
MCP server initialization update
backend/mcp_server.py
Replaces removed version/description args with an instructions=(...) argument in the FastMCP constructor call.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 In a burrow of code I neatly write,
Docs and scripts to make your backend light,
Start, check, record, then stop with care,
OmniVoice hums in the open air,
A tiny rabbit cheers—deploy tonight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: bundle Claude Code agent skill at .claude/skills/omnivoice/' directly and specifically describes the main change—adding a bundled Claude skill for OmniVoice at the specified directory path.
Description check ✅ Passed The description comprehensively covers all required sections: clear summary of the .gitignore change, detailed explanation of skill contents and capabilities, organized file layout, adherence to conventions, thorough test plan with specific results, cross-platform considerations, and explicit dependency on #112.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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

✨ 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 and usage tips.

@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: 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 @.claude/skills/omnivoice/scripts/start-backend.sh:
- Around line 9-10: The script sets URL via OMNIVOICE_API_URL but later
hardcodes port 3900 for binding and health checks; update the start-backend.sh
logic so the bind and probe use the configured URL instead of a fixed port.
Parse the port (and host) from the URL variable (falling back to 3900 if none),
construct the bind address and the probe URL from that parsed value, and replace
the hardcoded 3900 occurrences used in the backend start and the probe loops
(the blocks around the variables URL/OMNIVOICE_API_URL, and the probe sections
referenced by the earlier hardcoded lines 23-31 and 37-45) so the script starts
and polls the same endpoint. Ensure LOG usage remains unchanged.

In @.claude/skills/omnivoice/SKILL.md:
- Line 3: The skill description's "description" field currently claims "video
dubbing via the OmniVoice Studio MCP server" which contradicts the later note
that dubbing is not exposed via MCP; remove the unsupported MCP dubbing claim
from the description and any related triggers like "dub video" or "dubbing" in
the trigger list so the skill no longer advertises MCP-based dubbing, and
instead mention local dubbing only if desired; edit the SKILL.md "description"
string and the trigger tokens to keep claims consistent with the non-MCP dubbing
statement.

In `@backend/mcp_server.py`:
- Around line 51-54: The server-side instructions string (variable/invocation
named instructions in mcp_server.py) incorrectly advertises "video dubbing"
support; update that instructions text to reflect only the actual exposed
capabilities (e.g., voice cloning and voice design) or explicitly mention that
video dubbing is not provided by this MCP server so clients won't call
unsupported operations; locate the instructions assignment and edit the string
accordingly to match the real tool surface.
🪄 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 Plus

Run ID: 50419864-a403-4208-ac29-7bd7265f402f

📥 Commits

Reviewing files that changed from the base of the PR and between b34dcd9 and 7b8fe65.

📒 Files selected for processing (7)
  • .claude/skills/omnivoice/SKILL.md
  • .claude/skills/omnivoice/references/mcp-setup.md
  • .claude/skills/omnivoice/scripts/check-health.sh
  • .claude/skills/omnivoice/scripts/start-backend.sh
  • .claude/skills/omnivoice/scripts/stop-backend.sh
  • .gitignore
  • backend/mcp_server.py

Comment on lines +9 to +10
URL="${OMNIVOICE_API_URL:-http://127.0.0.1:3900}"
LOG="$REPO_ROOT/backend.log"

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make bind/check port consistent with configured API URL.

Line 9 allows overriding backend URL, but Lines 23-31 still hardcode port 3900. With a custom OMNIVOICE_API_URL, the script can start one port and probe another, then fail after 60s.

Suggested fix
 URL="${OMNIVOICE_API_URL:-http://127.0.0.1:3900}"
+PORT="${OMNIVOICE_PORT:-${URL##*:}}"
+PORT="${PORT%%/*}"

-if lsof -nP -iTCP:3900 -sTCP:LISTEN >/dev/null 2>&1; then
-  echo "port 3900 held but /health not responding — investigate before starting" >&2
-  lsof -nP -iTCP:3900 -sTCP:LISTEN >&2
+if lsof -nP -iTCP:"$PORT" -sTCP:LISTEN >/dev/null 2>&1; then
+  echo "port $PORT held but /health not responding — investigate before starting" >&2
+  lsof -nP -iTCP:"$PORT" -sTCP:LISTEN >&2
   exit 3
 fi

 cd "$REPO_ROOT"
-nohup uv run uvicorn main:app --app-dir backend --host 127.0.0.1 --port 3900 \
+nohup uv run uvicorn main:app --app-dir backend --host 127.0.0.1 --port "$PORT" \
   > "$LOG" 2>&1 &

Also applies to: 23-31, 37-45

🤖 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 @.claude/skills/omnivoice/scripts/start-backend.sh around lines 9 - 10, The
script sets URL via OMNIVOICE_API_URL but later hardcodes port 3900 for binding
and health checks; update the start-backend.sh logic so the bind and probe use
the configured URL instead of a fixed port. Parse the port (and host) from the
URL variable (falling back to 3900 if none), construct the bind address and the
probe URL from that parsed value, and replace the hardcoded 3900 occurrences
used in the backend start and the probe loops (the blocks around the variables
URL/OMNIVOICE_API_URL, and the probe sections referenced by the earlier
hardcoded lines 23-31 and 37-45) so the script starts and polls the same
endpoint. Ensure LOG usage remains unchanged.

Comment thread .claude/skills/omnivoice/SKILL.md Outdated
@@ -0,0 +1,98 @@
---
name: omnivoice
description: "Local TTS, voice cloning, voice design, and video dubbing via the OmniVoice Studio MCP server bundled in this repository. Open-source ElevenLabs alternative — nothing leaves the machine, 646 languages, runs on MPS/CUDA/ROCm/CPU. Use when: (1) generating speech from text in any of 646 languages, (2) cloning a voice from a 3-second reference clip, (3) designing a voice by gender/age/accent/pitch/style, (4) dubbing a video into another language, (5) listing voice profiles or personalities, (6) producing narration where privacy, cost, or absent API keys matter, (7) batch audio for blog posts or content pipelines. Triggers: 'omnivoice', 'voice clone', 'clone this voice', 'tts', 'narrate', 'generate speech', 'voice synthesis', 'dub video', 'voice design', 'local tts', 'multilingual voice', 'elevenlabs alternative'."

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove unsupported MCP capability claims from skill description.

Line 3 advertises “video dubbing via MCP server,” but Line 84 says dubbing is not exposed via MCP. This mismatch can cause agents to select this skill for unsupported tasks.

Suggested doc fix
-description: "Local TTS, voice cloning, voice design, and video dubbing via the OmniVoice Studio MCP server bundled in this repository. ... Triggers: ... 'dub video', ..."
+description: "Local TTS, voice cloning, and voice design via the OmniVoice Studio MCP server bundled in this repository. ... Triggers: ... "
🤖 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 @.claude/skills/omnivoice/SKILL.md at line 3, The skill description's
"description" field currently claims "video dubbing via the OmniVoice Studio MCP
server" which contradicts the later note that dubbing is not exposed via MCP;
remove the unsupported MCP dubbing claim from the description and any related
triggers like "dub video" or "dubbing" in the trigger list so the skill no
longer advertises MCP-based dubbing, and instead mention local dubbing only if
desired; edit the SKILL.md "description" string and the trigger tokens to keep
claims consistent with the non-MCP dubbing statement.

Comment thread backend/mcp_server.py
Comment on lines +51 to 54
instructions=(
"AI-agent interface for OmniVoice Studio — voice cloning, "
"voice design, and video dubbing in 646 languages."
),

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep MCP server instructions aligned with actual tool surface.

Lines 52-54 claim video dubbing support, but this server does not expose dubbing tools/resources. This can mislead MCP clients into calling unsupported operations.

Suggested fix
         instructions=(
-            "AI-agent interface for OmniVoice Studio — voice cloning, "
-            "voice design, and video dubbing in 646 languages."
+            "AI-agent interface for OmniVoice Studio — voice cloning, "
+            "voice design, and speech synthesis in 646 languages."
         ),
📝 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
instructions=(
"AI-agent interface for OmniVoice Studio — voice cloning, "
"voice design, and video dubbing in 646 languages."
),
instructions=(
"AI-agent interface for OmniVoice Studio — voice cloning, "
"voice design, and speech synthesis in 646 languages."
),
🤖 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 `@backend/mcp_server.py` around lines 51 - 54, The server-side instructions
string (variable/invocation named instructions in mcp_server.py) incorrectly
advertises "video dubbing" support; update that instructions text to reflect
only the actual exposed capabilities (e.g., voice cloning and voice design) or
explicitly mention that video dubbing is not provided by this MCP server so
clients won't call unsupported operations; locate the instructions assignment
and edit the string accordingly to match the real tool surface.

…helper

Two additions to the bundled skill, closing the gap where agents had no
procedural knowledge for creating a voice profile (the previous SKILL.md
said "use the UI or POST /profiles" but didn't include the recording +
trimming + verification workflow).

1. scripts/record-reference.sh — macOS-only helper that records a clean
   reference clip with **audible** countdown + start/stop cues via
   `say` + /System/Library/Sounds/Ping.aiff. Solves the buffering bug
   where text-mode "speak now" prompts arrive after recording starts.
   Captures a longer raw window then trims to ~10 sec of speech via
   silenceremove + atrim. Plays back for verification. Prints the
   next-step `curl` command for POST /profiles.

2. SKILL.md "Voice clone — end-to-end recipe" section (replaces the
   stub one-liner). Covers:
   - Path A: the bundled helper (one command, audible cues)
   - Path B: manual ffmpeg flow if the helper doesn't fit
   - POST /profiles multipart/form-data fields (required: name +
     ref_audio; optional: ref_text, language, instruct, seed, personality)
   - Reference clip quality factors that materially affect output
     (single speaker, natural prosody, 3-10 sec sweet spot, ref_text
     alignment, language correctness, loudness ≥ -15 dB peak)

Tested locally: recorded a 10-sec Spanish reference + 3-sec English
reference, created two profiles via the helper + curl flow, generated
14.1 sec of Spanish + 10.2 sec of English audio in the user's cloned
voice. Round-trip works end-to-end at steps=16 on Apple Silicon MPS.

Frontmatter description unchanged (860 chars, under the 1024 limit).
Body grew from ~120 to 169 lines (still well under the 500-line skill
ceiling).

@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.

♻️ Duplicate comments (1)
.claude/skills/omnivoice/SKILL.md (1)

3-3: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove unsupported MCP capability claims from skill description.

The description advertises "video dubbing via the OmniVoice Studio MCP server" and includes "dub video" as a trigger, but Line 149 explicitly states dubbing is not exposed via MCP (web UI only). This mismatch can mislead agents into selecting this skill for unsupported tasks.

📝 Proposed fix
-description: "Local TTS, voice cloning, voice design, and video dubbing via the OmniVoice Studio MCP server (open-source ElevenLabs alternative; nothing leaves the machine, runs on MPS/CUDA/CPU). Use when: (1) generating speech from text in any of 646 languages, (2) cloning a voice from a 3-second reference clip, (3) designing a voice by gender/age/accent/pitch/style, (4) dubbing a video into another language, (5) listing voice profiles or personality presets, (6) producing narration where privacy, cost, or absent API keys matter, (7) non-English narration where Edge TTS/kokoro fall short, (8) batch audio for blog posts or content pipelines. Triggers: 'omnivoice', 'voice clone', 'clone this voice', 'tts', 'narrate', 'generate speech', 'voice synthesis', 'dub video', 'voice design', 'local tts', 'multilingual voice', 'narrate this post', 'elevenlabs alternative'."
+description: "Local TTS, voice cloning, and voice design via the OmniVoice Studio MCP server (open-source ElevenLabs alternative; nothing leaves the machine, runs on MPS/CUDA/CPU). Use when: (1) generating speech from text in any of 646 languages, (2) cloning a voice from a 3-second reference clip, (3) designing a voice by gender/age/accent/pitch/style, (4) listing voice profiles or personality presets, (5) producing narration where privacy, cost, or absent API keys matter, (6) non-English narration where Edge TTS/kokoro fall short, (7) batch audio for blog posts or content pipelines. Triggers: 'omnivoice', 'voice clone', 'clone this voice', 'tts', 'narrate', 'generate speech', 'voice synthesis', 'voice design', 'local tts', 'multilingual voice', 'narrate this post', 'elevenlabs alternative'."
🤖 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 @.claude/skills/omnivoice/SKILL.md at line 3, The skill description's
description string and trigger list claim "video dubbing via the OmniVoice
Studio MCP server" and include the "dub video" trigger while MCP does not expose
dubbing; update the SKILL.md description text and trigger list to remove or
reword any MCP-related dubbing claims (remove "video dubbing via the OmniVoice
Studio MCP server" and the "dub video" trigger or change to "video dubbing (web
UI only)") so the description and triggers match the actual capability exposed
by the MCP server and the note that dubbing is web-UI-only.
🤖 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.

Duplicate comments:
In @.claude/skills/omnivoice/SKILL.md:
- Line 3: The skill description's description string and trigger list claim
"video dubbing via the OmniVoice Studio MCP server" and include the "dub video"
trigger while MCP does not expose dubbing; update the SKILL.md description text
and trigger list to remove or reword any MCP-related dubbing claims (remove
"video dubbing via the OmniVoice Studio MCP server" and the "dub video" trigger
or change to "video dubbing (web UI only)") so the description and triggers
match the actual capability exposed by the MCP server and the note that dubbing
is web-UI-only.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8dd2319b-a98d-4db2-a685-dba4da04f311

📥 Commits

Reviewing files that changed from the base of the PR and between 7b8fe65 and 9b84c1f.

📒 Files selected for processing (2)
  • .claude/skills/omnivoice/SKILL.md
  • .claude/skills/omnivoice/scripts/record-reference.sh

Adversarial multi-agent review (code + comment + silent-failure analyzers
on parallel reviewers) surfaced one blocker, one critical silent-failure
class, two medium-severity bugs, and two minor doc inaccuracies. All
addressed in this commit.

Blocker (cited 3x by both code-reviewer and comment-analyzer):
- SKILL.md linked references/engines-comparison.md three times (lines 44,
  153, 160) but the file was never copied into the upstream skill tree.
  + Added the file (engine decision tree across OmniVoice / kokoro /
    Voicebox / Edge TTS / ElevenLabs / cloud APIs).

Critical — record-reference.sh (was 4/10):
- Mic-permission silent failure: macOS denies the mic by sending a silent
  stream; ffmpeg exits 0 with a valid silent WAV. The script printed
  "✓ raw captured" and produced a degenerate reference clip that would
  train a broken voice profile.
  + Parse mean_volume from volumedetect; exit 3 with a diagnostic
    pointing the user to System Settings → Privacy → Microphone if
    the recording is below -50 dB.
- afplay backgrounded with no exit check; if /System/Library/Sounds/*.aiff
  is missing the user gets no audible cue.
  + beep() helper falls back to printf '\a' (terminal bell) when the
    system sound file is missing.
- silenceremove silent corruption: silent input → near-empty output WAV,
  exit 0.
  + ffprobe duration check after trim; exit 4 if < 2.0 sec.
- trap only covered EXIT; Ctrl-C / SIGTERM mid-recording leaked tmp file.
  + trap '...' EXIT INT TERM HUP.
- macOS guard ran after mktemp + trap.
  + Moved guard to first executable line.
- afplay verification swallowed stderr.
  + Drop 2>/dev/null; surface failure as a warning.
- Documented exit codes in header (0/2/3/4).

Medium — start-backend.sh (was 6/10):
- TOCTOU race: lsof check → uvicorn start could lose the port to another
  process; only signal was a 60s health timeout.
  + Added `kill -0 $PID` check inside the probe loop; immediate exit 5
    with log tail if uvicorn died.
- lsof check couldn't tell "stale us" from "third party" — same exit 3
  for both.
  + ps -o command attribution; the message now tells the user whether
    it's a stale uvicorn (suggest stop-backend.sh) or unknown process.
- Documented exit codes (0/2/3/4/5).

Medium — stop-backend.sh (was 7/10):
- No post-SIGKILL verification — script exited 0 even if process still
  bound.
  + Added current_pids() helper; re-query after SIGKILL; exit 1 if still
    bound, with lsof dump for diagnostics.
- 2>/dev/null || true on kill swallowed EPERM silently.
  + Capture stderr; classify EPERM vs ESRCH; exit 2 on EPERM with
    actionable hint (try sudo).
- Documented exit codes (0/1/2).

Minor docs (comment-analyzer):
- SKILL.md line 120 claimed profiles persist as `<id>.wav`. Actual
  backend (profiles.py:48-50) preserves uploaded extension.
  + Reworded to `<id>.<ext>` with explanation.
- mcp-setup.md line 68 cited HF cache path as Linux/macOS only.
  Windows redirects via backend/core/config.py:38 to
  %LOCALAPPDATA%\OmniVoice\hf_cache.
  + Added Windows row + reference to config.py.

Re-validated: all 6 files compile under set -euo pipefail; SKILL.md
frontmatter description stays at 860 chars (under 1024 cap); skill body
under 500 lines.

Diff: 6 files changed, ~+269/-47.
@broomva

broomva commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

P20 cross-review verdict

Ran a 3-agent adversarial review on the original commits (code-reviewer + comment-analyzer + silent-failure-hunter) to enforce a "writer cannot be final judge" gate. Findings + remediation in commit 5d3d188:

Reviewer Original Findings After fix
code-reviewer 7/10 C1: missing references/engines-comparison.md linked 3× (SKILL.md:44, 153, 160) added the file
comment-analyzer 8/10 Profile-path <id>.wav claim wrong (actual: <id>.<uploaded-ext>); HF cache path missing Windows footnote both fixed
silent-failure-hunter 4/10 FAIL mic-permission silent failure (denied mic → silent WAV → degenerate clone); afplay silent if system sound missing; silenceremove silent corruption (no duration check); trap covered EXIT only; verification swallowed stderr; OS guard out of order; exit codes incoherent 9/10 PASS after re-score: all 7 items addressed with machine-checkable behavior (parsed thresholds, distinct exit codes, fallback cues, expanded trap, actionable diagnostics)

Same fixes mirrored in the standalone skill at broomva/omnivoice-skill@ffccf18 so both install paths stay in sync.

The original critical finding (mic permission denied → silent WAV → broken clone with no diagnostic path) matches the exact "first-run that actually works" failure mode flagged as P0 in this repo's own CLAUDE.md. Fixing it before merge is what the P20 gate is for.

@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 @.claude/skills/omnivoice/references/engines-comparison.md:
- Line 7: The two unlabeled fenced code blocks containing the decision tree text
(the block starting with "Is voice cloning required?" and the later block
starting with "research → narrative → visual assets → AUDIO (OmniVoice) → video
assembly → distribution") must be updated to include a language tag (e.g., text)
after the opening triple backticks to satisfy markdown linting (MD040); locate
the opening ``` for each block and change it to ```text so both fenced blocks
are labeled.
🪄 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 Plus

Run ID: 14cc946c-9dc8-4ce7-b0b3-5db35c51f0d6

📥 Commits

Reviewing files that changed from the base of the PR and between 9b84c1f and 5d3d188.

📒 Files selected for processing (6)
  • .claude/skills/omnivoice/SKILL.md
  • .claude/skills/omnivoice/references/engines-comparison.md
  • .claude/skills/omnivoice/references/mcp-setup.md
  • .claude/skills/omnivoice/scripts/record-reference.sh
  • .claude/skills/omnivoice/scripts/start-backend.sh
  • .claude/skills/omnivoice/scripts/stop-backend.sh


## Decision tree

```

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add fenced code block languages to satisfy markdown linting.

Line 7 and Line 60 use unlabeled fenced blocks; adding a language (e.g., text) avoids MD040 warnings.

Suggested patch
-```
+```text
 Is voice cloning required?
 ├─ yes → OmniVoice (3-sec ref clip, zero-shot, 646 langs)
 └─ no →
@@
-```
+```text
@@
-```
+```text
 research → narrative → visual assets → AUDIO (OmniVoice) → video assembly → distribution
-```
+``` 

Also applies to: 60-60

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 7-7: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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 @.claude/skills/omnivoice/references/engines-comparison.md at line 7, The two
unlabeled fenced code blocks containing the decision tree text (the block
starting with "Is voice cloning required?" and the later block starting with
"research → narrative → visual assets → AUDIO (OmniVoice) → video assembly →
distribution") must be updated to include a language tag (e.g., text) after the
opening triple backticks to satisfy markdown linting (MD040); locate the opening
``` for each block and change it to ```text so both fenced blocks are labeled.

@debpalash

Copy link
Copy Markdown
Owner

/ecc-tools analyze

@ecc-tools

ecc-tools Bot commented May 27, 2026

Copy link
Copy Markdown

Analysis Queued

Analyzing debpalash/OmniVoice-Studio on pull request head 5d3d18815ee6

Analysis Pipeline
Stage Description Status
Compare Snapshot Resolving PR head/base diff before branch fallback pending
Commit History Fetching up to 200 commits (queued-mode cap) pending
File Sampling Extracting code patterns and structure pending
Pattern Detection AI analysis of conventions and workflows pending
Bundle Generation Creating skills, rules, commands, identity, and instincts pending
Pull Request Opening PR with generated files pending

Estimated time: 2-5 minutes depending on repository size.


10/10 analyses remaining this month (free tier) | ECC Tools

@debpalash

Copy link
Copy Markdown
Owner

@greptileai

@greptile-apps

greptile-apps Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a Claude Code agent skill bundle at .claude/skills/omnivoice/ and narrows the .gitignore so the skill directory is tracked while ad-hoc Claude state is still ignored. The companion backend/mcp_server.py change fixes a TypeError introduced by mcp SDK ≥ 1.10 dropping the version/description constructor kwargs in favour of instructions.

  • .gitignore.claude/ whole-directory ignore replaced with .claude/* + two negations; pattern is correct and self-consistent.
  • Skill bundleSKILL.md, two reference docs, and four helper scripts added; record-reference.sh correctly uses mktemp+trap and a platform guard, but stop-backend.sh uses a predictable fixed path /tmp/.omnivoice-kill-err instead, and both start-backend.sh and stop-backend.sh hardcode port 3900 in their lsof/uvicorn commands while start-backend.sh reads the port from $OMNIVOICE_API_URL for health checks, creating an inconsistency for non-default port configurations.
  • backend/mcp_server.py — Correct fix for mcp SDK ≥ 1.10; verifying the lockfile pins mcp[cli] to ≥ 1.10 is recommended before merging.

Confidence Score: 4/5

Safe to merge; all changes are additive (new docs + scripts) except the one-line MCP server fix, which correctly addresses a known SDK breaking change.

The .gitignore tweak and skill documentation are low-risk. The mcp_server.py fix is a correct response to an upstream SDK breaking change. The helper scripts are non-default opt-in tools, so issues in them don't affect any running feature. The findings in the lifecycle scripts — a fixed temp-file path in stop-backend.sh and mismatched port handling between health checks and lsof/uvicorn commands — would cause silent misbehaviour only when the backend runs on a non-default port or when two script instances race, but neither breaks any default workflow today.

start-backend.sh and stop-backend.sh — port handling inconsistency between $OMNIVOICE_API_URL and the hardcoded 3900 in lsof and uvicorn --port commands.

Important Files Changed

Filename Overview
.gitignore Narrows .claude/ ignore from whole-directory to glob + negation, correctly untracking only the skill bundle. Pattern is well-formed.
backend/mcp_server.py Drops deprecated version/description kwargs in favour of instructions for mcp SDK ≥ 1.10 compatibility; correct fix but mcp[cli] version is not pinned in this diff so old-SDK users will now break on instructions instead.
.claude/skills/omnivoice/SKILL.md New skill index with frontmatter, tool table, workflow examples, and decision notes. Well-structured and within the 500-line limit.
.claude/skills/omnivoice/scripts/start-backend.sh Idempotent uvicorn boot script; port 3900 is hardcoded in both the lsof collision check and the uvicorn launch command, while health verification reads from $OMNIVOICE_API_URL, creating an inconsistency that silently misfires when the env var points to a different port.
.claude/skills/omnivoice/scripts/stop-backend.sh Graceful-then-SIGKILL shutdown script; uses a predictable fixed temp-file path /tmp/.omnivoice-kill-err instead of mktemp, and port 3900 is hardcoded with no env-var override, so the script silently exits 0 without stopping the backend if it is running on a non-default port.
.claude/skills/omnivoice/scripts/check-health.sh Clean, minimal health-check wrapper that correctly reads $OMNIVOICE_API_URL and exits 0/1.
.claude/skills/omnivoice/scripts/record-reference.sh macOS-only recording helper with platform guard, mktemp-based temp file, trap cleanup, silence detection via awk, and audible cues. Correctly scoped as opt-in.

Sequence Diagram

sequenceDiagram
    participant Agent as Claude Agent
    participant Skill as skills CLI / CLAUDE.md
    participant MCP as MCP Server (mcp_server.py)
    participant Backend as FastAPI Backend (127.0.0.1:3900)
    participant HF as HuggingFace Hub

    Note over Skill: npx skills add debpalash/OmniVoice-Studio
    Skill-->>Agent: loads .claude/skills/omnivoice/SKILL.md

    Agent->>Agent: start-backend.sh
    Agent->>Backend: lsof -iTCP:3900 (port collision check)
    Agent->>Backend: nohup uvicorn --port 3900
    loop 60s health probe
        Agent->>Backend: GET $OMNIVOICE_API_URL/health
        Backend-->>Agent: "{status:ok, device:mps|cuda|cpu}"
    end

    Agent->>MCP: spawn via stdio (uv run python -m backend.mcp_server)
    MCP->>Backend: "FastMCP(instructions=...) registered"

    Agent->>MCP: generate_speech(text, profile_id, language, steps)
    MCP->>Backend: POST /generate
    Backend->>HF: download k2-fsa/OmniVoice (~2.4 GB, first call only)
    HF-->>Backend: model weights cached
    Backend-->>MCP: "{wav_base64, audio_id, generation_time_s}"
    MCP-->>Agent: JSON result

    Agent->>Agent: stop-backend.sh
    Agent->>Backend: kill -TERM pid
    Backend-->>Agent: graceful shutdown
Loading

Comments Outside Diff (3)

  1. .claude/skills/omnivoice/scripts/stop-backend.sh, line 639-650 (link)

    P2 Fixed temp-file path for SIGTERM error capture

    /tmp/.omnivoice-kill-err is a predictable, globally shared path. Two concurrent invocations of this script overwrite each other's captured kill stderr, and on a multi-user host a symlink pre-created at that path will be followed by the 2> redirect, potentially truncating an arbitrary file. Compare record-reference.sh, which correctly uses mktemp -t omnivoice-raw-XXXXX with a trap 'rm -f "$RAW"' EXIT INT TERM HUP — the same pattern should be applied here.

    Fix in Claude Code

  2. .claude/skills/omnivoice/scripts/start-backend.sh, line 549-566 (link)

    P2 Port 3900 hardcoded in lsof and uvicorn but not for health checks

    URL is derived from $OMNIVOICE_API_URL (line 549) and used for both the "already running?" poll (line 559) and the post-start health loop (line 598). However, both the lsof -iTCP:3900 collision guard (line 566) and the uvicorn --port 3900 launch command (line 583) hardcode port 3900. If a user sets OMNIVOICE_API_URL to a non-default port, the health checks target that port while uvicorn still starts on 3900, so the 60-second wait always times out with exit code 4. Deriving PORT from $URL and using it in both the lsof call and the uvicorn --port argument would make the script self-consistent.

    Fix in Claude Code

  3. .claude/skills/omnivoice/scripts/stop-backend.sh, line 624-625 (link)

    P2 stop-backend.sh always targets port 3900 with no env-var override

    Unlike check-health.sh and start-backend.sh, this script reads no OMNIVOICE_API_URL variable — the lsof -iTCP:3900 call and the current_pids helper are fully hardcoded. A user whose backend runs on a different port would see no listener on 3900 and a silent exit 0, leaving the backend running. Adding a PORT variable derived from OMNIVOICE_API_URL and substituting it in current_pids would keep the three lifecycle scripts consistent.

    Fix in Claude Code

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(skill): address P20 cross-review fin..." | Re-trigger Greptile

@debpalash
debpalash merged commit fa9c7d4 into debpalash:main May 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants