Repository navigation
fix(cli): turn off onnxruntime telemetry for Kokoro synthesis - #5425
jrusso1020 merged 1 commit into
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at a4e8b4e7. Thanks for the clear write-up in #5424.
What I checked
- The switch is real in onnxruntime 1.30.0. At the
v1.30.0tag,core/platform/posix/telemetry.ccchecksIsTelemetryDisabledByEnvironment()when the 1DS provider starts up. WithORT_DISABLE_TELEMETRYset to a truthy value, it returns before the uploader is created, and the decision holds for the life of the process.docs/Privacy.mddocuments the same thing: set it "before ONNX Runtime initializes". Setting it in the child's environment satisfies that, because it's in place before Python imports onnxruntime.ort.disable_telemetry_events()wouldn't be enough here, since it only suppresses non-essential events after the SDK is already running. - One place covers every Kokoro path.
media-use/lib/tts-local-provider.mjsandskills/media-use/audio/scripts/lib/tts.mjsboth shell out tohyperframes tts, so they both reach thisexecFileSync. - The rest of the environment passes through.
{ ...process.env, ... }keepsHYPERFRAMES_PYTHON,PATHand the rest. - The test fails without the fix. All 31 tests in
src/tts/pass at this head. With theenvline removed, the new test fails with "expected undefined to be '1'".
Nit (not blocking): hasPythonPackage(python, "kokoro_onnx") runs import kokoro_onnx without the variable, and that loads onnxruntime too. I haven't checked whether importing alone starts the uploader. That's a short-lived process, though, so the exit race is much less likely there, and if it did crash the user would get a loud "not installed" error rather than a dropped line. Passing the same env there would rule it out.
No conflicts with main. When I posted this, CI showed 20 passed and 16 still running.
I didn't run this on macOS or reproduce the SIGABRT.
— Rames
|
Thanks for this! It's approved and CI is green, but Quickest fix with an SSH key you already use for GitHub: git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub # your public key
git config --global commit.gpgsign true
git commit --amend --no-edit -S
git push --force-with-leaseAlso add that same key to GitHub as a Signing key (Settings → SSH and GPG keys → New SSH key → Key type: Signing key), and make sure the commit's email is a verified email on your account. Docs: Signing commits and Telling Git about your signing key. Once the commit shows Verified, we'll re-approve and merge. — Rames |
onnxruntime's 1DS telemetry, on by default in its macOS wheels, can abort the Python process at exit when an upload response races teardown. execFileSync then throws for a line whose WAV was already written, and the audio skill drops it as non-fatal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Head branch was pushed to by a user without write access
a4e8b4e to
dd0c18a
Compare
|
Thanks for the thorough review, Rames! The commit is now signed and shows as Verified. I'll leave the |
What
Run the Kokoro synthesis subprocess with
ORT_DISABLE_TELEMETRY=1.Why
onnxruntime's 1DS telemetry is on by default in its macOS Python wheels. When an upload response arrives while the Python process is exiting, the 1DS HTTP worker locks a static mutex that exit already destroyed and the process aborts, after the WAV was written.
execFileSyncthrows,hyperframes ttsexits 1, and the audio skill drops the line as a non-fatal anomaly, so narration can silently lose a sentence. Details, crash stacks and evidence in #5424.Related work
Fixes #5424. Related: #2775 (narration dropped through a different cause).
How
One line:
env: { ...process.env, ORT_DISABLE_TELEMETRY: "1" }on theexecFileSynccall inpackages/cli/src/tts/synthesize.ts. onnxruntime reads the variable once when it loads, so the 1DS SDK never starts in the synth process; the rest of the environment is passed through unchanged (HYPERFRAMES_PYTHON,PATH, etc. are unaffected). The explanation lives here rather than in a code comment, per the comment-share ratchet.Test plan
synthesize.cache.test.tsasserts the synth call passesORT_DISABLE_TELEMETRY: "1". It fails onmain("expected undefined to be '1'") and passes with the change; all 31 tests inpackages/cli/src/tts/pass (vitest 4.1.11).check-comment-citations.mjsOK andcomment-ratchet.mjsat or under the base share.🤖 Generated with Claude Code