ci: gate omnivoice-tts build to pin changes; drop hanging Intel-Mac leg - #147
Conversation
The omnivoice-tts C++ runtime is pinned to a commit SHA in quant_map.json, so it only needs rebuilding when that pin (or the build script) changes — not on every PR/push. Running it per-push left the heavily-contended hosted macOS runners (esp. Intel macos-13) sitting in "Waiting for a runner…" for hours as a perpetual queued check (the UNSTABLE state on every PR). - Moved the build out of ci.yml into its own workflow, .github/workflows/build-omnivoice-tts.yml, gated to: paths [quant_map.json, scripts/build-omnivoice-tts.sh, the workflow] + workflow_dispatch. Normal PRs no longer trigger (or hang on) it. - Dropped the Intel darwin-x86_64 (macos-13) matrix leg: that hosted pool is unusably contended and Apple's momentum is on arm64; Intel-Mac users get the in-process OmniVoiceBackend fallback (already the documented behavior). Kept linux-x86_64, windows-x86_64, darwin-arm64. Re-add macos-13 here if first-class Intel binaries are ever needed. Both workflows YAML-validated. Matches ci.yml's stated philosophy of keeping heavy platform builds off the per-PR path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR separates the ChangesCI Workflow Refactoring
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| .github/workflows/build-omnivoice-tts.yml | New workflow extracted from ci.yml, now gated to path filters; adds timeout-minutes, SHA validation, persist-credentials: false, and env-var injection hardening. No new issues found. |
| .github/workflows/ci.yml | Removes the build-omnivoice-tts job (79 lines) that ran on every push; no other changes to ci.yml logic. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Git event] --> B{Event type?}
B -->|push / pull_request to main| C{Path filter match?}
B -->|workflow_dispatch| F[Run build-omnivoice-tts.yml]
C -->|quant_map.json, build script,\nor workflow file changed| F
C -->|No matching paths| G[Skip — workflow not triggered]
F --> H[Checkout with persist-credentials: false]
H --> I[Read + validate runtime_commit_sha\nregex: ^0-9a-fA-F 7,40 $]
I -->|invalid SHA| J[::error:: + exit 1]
I -->|valid SHA| K{Matrix platform?}
K -->|linux-x86_64| L[apt-get install libopenblas-dev]
K -->|windows-x86_64| M[No extra deps]
K -->|darwin-arm64\nexperimental: true| N[No extra deps]
L --> O[build-omnivoice-tts.sh via env vars]
M --> O
N --> O
O --> P[Upload binary artifact]
P --> Q{All non-experimental jobs passed?}
Q -->|Yes| R[Workflow success]
Q -->|darwin-arm64 failed\ncontinue-on-error: true| R
Reviews (2): Last reviewed commit: "ci: timeout-minutes + injection-harden t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/build-omnivoice-tts.yml:
- Around line 68-69: The workflow injects runtime_commit_sha from quant_map.json
into a shell context unsafely (sha and steps.pin.outputs.sha), risking shell
injection; fix by emitting a safely shell-escaped/quoted value from the python
step (e.g., use json.dumps or Python's shlex.quote/print with proper quoting) so
the GITHUB_OUTPUT receives a quoted/escaped sha, and ensure every use in the
workflow is wrapped in double quotes when interpolating (e.g., "${{
steps.pin.outputs.sha }}") to prevent argument-splitting or execution.
- Line 61: Replace the floating action refs with SHA-pinned commits: update
occurrences of actions/checkout@v4 and actions/upload-artifact@v4 in the
workflow to use the corresponding full commit SHAs for those actions (e.g.,
actions/checkout@<commit-sha> and actions/upload-artifact@<commit-sha>),
ensuring you fetch the official commit SHA from the actions' GitHub
repos/releases and commit the updated workflow to lock the exact versions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c4637471-8ab0-496e-a2ce-8ea4a9bf14aa
📒 Files selected for processing (2)
.github/workflows/build-omnivoice-tts.yml.github/workflows/ci.yml
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yml
| # block — the SPIKE-01 ADR records the in-process fallback. | ||
| experimental: true | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether all workflow action refs are pinned to full commit SHAs.
# Expected: every "uses:" ref ends with @<40-hex SHA>
file=".github/workflows/build-omnivoice-tts.yml"
echo "Checking $file"
rg -nP '^\s*(uses:|- uses:)\s*\S+@\S+' "$file"
python - <<'PY'
import re, pathlib
p = pathlib.Path(".github/workflows/build-omnivoice-tts.yml")
bad = []
for i, line in enumerate(p.read_text().splitlines(), 1):
m = re.search(r'^\s*(?:-\s*)?uses:\s*([^\s]+)@([^\s]+)\s*$', line)
if not m:
continue
ref = m.group(2)
if not re.fullmatch(r'[0-9a-fA-F]{40}', ref):
bad.append((i, line.strip()))
if bad:
print("Non-SHA-pinned actions found:")
for i, l in bad:
print(f"Line {i}: {l}")
else:
print("All actions are SHA pinned.")
PYRepository: debpalash/OmniVoice-Studio
Length of output: 311
Pin GitHub Actions to commit SHAs in .github/workflows/build-omnivoice-tts.yml (avoid floating @v4 tags).
Lines 61 and 86 use non-SHA-pinned refs:
actions/checkout@v4actions/upload-artifact@v4
🔧 Suggested pattern
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<full_commit_sha_for_v4>
@@
- - name: Upload binary artifact
- uses: actions/upload-artifact@v4
+ - name: Upload binary artifact
+ uses: actions/upload-artifact@<full_commit_sha_for_v4>🧰 Tools
🪛 zizmor (1.25.2)
[error] 61-61: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/build-omnivoice-tts.yml at line 61, Replace the floating
action refs with SHA-pinned commits: update occurrences of actions/checkout@v4
and actions/upload-artifact@v4 in the workflow to use the corresponding full
commit SHAs for those actions (e.g., actions/checkout@<commit-sha> and
actions/upload-artifact@<commit-sha>), ensuring you fetch the official commit
SHA from the actions' GitHub repos/releases and commit the updated workflow to
lock the exact versions.
There was a problem hiding this comment.
Declining the SHA-pin here, consistent with the call on #135: the repo's convention is floating major tags across ci.yml and release.yml, so pinning only this one file would be inconsistent, and piecemeal SHA pins rot without Dependabot to bump them. Worth doing — but as a repo-wide hardening pass + github-actions Dependabot, not in this CI-fix PR.
There was a problem hiding this comment.
Understood — that's a reasonable call. Consistent conventions across workflow files are easier to audit, and SHA pins without an automated bump mechanism (Dependabot or Renovate) tend to drift and become a maintenance burden of their own.
Would you like me to open a follow-up issue to track the repo-wide hardening pass (pinning all github-actions refs to SHAs + enabling github-actions Dependabot), so it doesn't get lost?
✏️ Learnings added
Learnt from: debpalash
Repo: debpalash/OmniVoice-Studio PR: 147
File: .github/workflows/build-omnivoice-tts.yml:64-64
Timestamp: 2026-05-29T08:33:41.879Z
Learning: In the `debpalash/OmniVoice-Studio` repository, the convention across GitHub Actions workflow files (`.github/workflows/`) is to use floating major-version tags (e.g., `v4`) rather than pinning actions to full commit SHAs. A repo-wide hardening pass (SHA-pinning + enabling `github-actions` Dependabot) has been deferred to a future dedicated PR. Do not flag individual workflow files for not using SHA-pinned action refs, as the inconsistency is intentional until that hardening pass is done. See PR `#147` and PR `#135` for context.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
…eview) - Greptile: add `timeout-minutes: 45` so a hung leg (esp. experimental darwin-arm64 Metal) can't run to GitHub's 6h ceiling — same resource-drain class this PR addresses. - CodeRabbit: stop interpolating the pinned SHA / platform directly into the run block. Validate the SHA is a git hash in the pin step, then pass it + platform via quoted env vars (no shell-injection surface from quant_map.json). Declined: SHA-pinning actions@v4 — matches the repo's floating-tag convention (ci.yml/release.yml); belongs in a repo-wide hardening pass + Dependabot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 @.github/workflows/build-omnivoice-tts.yml:
- Line 74: The workflow's SHA validation uses the regex ^[0-9a-fA-F]{7,40}$ in
the if ! [[ "$sha" =~ ... ]] check, but the build script expects exactly 40 hex
chars (^[0-9a-fA-F]{40}$), causing short SHAs to pass here but fail later;
update the workflow regex to ^[0-9a-fA-F]{40}$ so the "if ! [[ \"$sha\" =~ ...
]]" check enforces a full 40-character hex SHA to match the build script's
validation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a984376-9897-43a0-9dae-a9a6a3d108d8
📒 Files selected for processing (1)
.github/workflows/build-omnivoice-tts.yml
| sha=$(python -c "import json; print(json.load(open('backend/engines/omnivoice_gguf/quant_map.json'))['_meta']['runtime_commit_sha'])") | ||
| # Validate it's a git SHA before it ever reaches a shell command — | ||
| # a crafted quant_map.json value must not be able to inject shell. | ||
| if ! [[ "$sha" =~ ^[0-9a-fA-F]{7,40}$ ]]; then |
There was a problem hiding this comment.
Validation regex mismatch with build script.
The workflow validates the SHA with ^[0-9a-fA-F]{7,40}$ (7–40 hex chars), but the build script at scripts/build-omnivoice-tts.sh line 48 requires exactly 40 characters: ^[0-9a-fA-F]{40}$. A short SHA (e.g., 7–39 chars) will pass here but fail the build step.
🔒 Proposed fix: align with build script validation
- if ! [[ "$sha" =~ ^[0-9a-fA-F]{7,40}$ ]]; then
+ if ! [[ "$sha" =~ ^[0-9a-fA-F]{40}$ ]]; then
echo "::error::runtime_commit_sha is not a valid git SHA: '$sha'"🤖 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 @.github/workflows/build-omnivoice-tts.yml at line 74, The workflow's SHA
validation uses the regex ^[0-9a-fA-F]{7,40}$ in the if ! [[ "$sha" =~ ... ]]
check, but the build script expects exactly 40 hex chars (^[0-9a-fA-F]{40}$),
causing short SHAs to pass here but fail later; update the workflow regex to
^[0-9a-fA-F]{40}$ so the "if ! [[ \"$sha\" =~ ... ]]" check enforces a full
40-character hex SHA to match the build script's validation.
1 participant
Fixes the perpetually-hanging
Build omnivoice-tts (darwin-x86_64)check that sits in "Waiting for a runner…" for hours on every PR.Root cause
The
omnivoice-ttsC++ runtime is pinned to a commit SHA inquant_map.json, yet its 4-platform build ran on every PR + push to main. The hosted macOS runners — especially the Intelmacos-13pool — are heavily contended, so that leg queued for hours. It'scontinue-on-errorso it never blocked merge, but it kept every PR in theUNSTABLEstate with a stuck "queued" check.Fix
.github/workflows/build-omnivoice-tts.yml) gated to: changes toquant_map.json/scripts/build-omnivoice-tts.sh/ the workflow itself, +workflow_dispatch. Normal PRs no longer trigger it → no more hanging check, faster CI. It still rebuilds automatically when the pin changes (its whole purpose).darwin-x86_64(macos-13) leg — unusably contended, and Intel-Mac users already fall back to the in-processOmniVoiceBackend(documented behavior). Keptlinux-x86_64,windows-x86_64,darwin-arm64. Easy to re-add if you ever want first-class Intel binaries.This matches
ci.yml's own stated philosophy: "keep the heavy platform build off the per-PR path."Both workflows YAML-validated.
🤖 Generated with Claude Code
Summary by CodeRabbit