Skip to content

[Repo Assist] fix(release): enforce minimum VC++ runtime version for onnxruntime/sherpa-onnx - #712

Closed
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/fix-issue-703-vc-runtime-min-version-8f635a961f87b03a
Closed

[Repo Assist] fix(release): enforce minimum VC++ runtime version for onnxruntime/sherpa-onnx#712
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/fix-issue-703-vc-runtime-min-version-8f635a961f87b03a

Conversation

@github-actions

@github-actions github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Closes #703

Summary

Adds a minimum VC++ runtime file-version check to Test-ReleaseNativeDependencies.ps1. VCRuntime.CefSharp.140 1.0.5 ships vcruntime140.dll version 14.29 (VS 2019 16.11), but org.k2fsa.sherpa.onnx 1.13 and Microsoft.ML.OnnxRuntime 1.26 are compiled with VS 2022 and need ≥ 14.40. When the older app-local DLL wins the loader search over the user's system runtime, sherpa-onnx-c-api fails DLL initialisation (0x8007045A) during GC finalisation of OfflineTts, crash-looping the tray.

Root Cause

Test-ReleaseNativeDependencies.ps1 verified presence and Authenticode signature of the app-local VC runtime DLLs, but never checked their file version. A stale-version DLL could ship undetected and only fail at runtime.

Fix

scripts/Test-ReleaseNativeDependencies.ps1

  • Add $Script:MinVCRuntimeVersion = [Version]"14.40.0.0" constant (documented with issue reference).
  • In the existing app-local VC runtime check loop, read [System.Diagnostics.FileVersionInfo] for vcruntime140.dll on Windows and emit a clear, actionable error when the version is below the minimum.
  • Update the script description to document the multi-library VC runtime dependency.

The check runs only on Windows (FileVersionInfo reads Windows PE metadata); it is skipped on Linux/macOS CI legs where the DLL cannot be introspected. On the Windows x64/arm64 build legs where RequireAppLocalVCRuntime is used, the error message directs maintainers to update VCRuntime.CefSharp.140 to a version shipping 14.40+ DLLs.

tests/OpenClaw.Tray.Tests/ReleaseSigningWorkflowTests.cs

  • Add ReleaseWorkflow_VCRuntimeMinimumVersionIsEnforced test that reads the verifier script and asserts it contains the minimum version constant, FileVersionInfo usage, and the issue reference — ensuring the version gate cannot be silently removed.

Trade-offs

  • The version check does not fire on the Linux runner (where the CI build runs today); it would fire on Windows in CI once the packaging includes the outdated runtime. Maintainers would then need to update VCRuntime.CefSharp.140 to a version that ships 14.40+ DLLs.
  • We do not bundle a newer VC runtime package directly in this PR because the correct package version cannot be verified without Windows access and NuGet connectivity. That is the follow-up packaging work.

Test Status

  • Tray tests: ✅ 957 passed, 0 failed, 2 skipped (was 956 before; +1 new test)
  • Shared tests: ✅ 2041 passed, 8 pre-existing failures (Windows path / networking tests unrelated to this change), 29 skipped

Generated by 🌈 Repo Assist, see workflow run. Learn more.

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

…erpa-onnx

Adds a file-version check for vcruntime140.dll in the release native-dependency
verifier script. VCRuntime.CefSharp.140 1.0.5 ships 14.29 (VS 2019 16.11), but
OnnxRuntime 1.26 and sherpa-onnx 1.13 are compiled with VS 2022 and need 14.40+.
Bundling 14.29 causes sherpa-onnx-c-api to fail DLL initialisation (0x8007045A),
crash-looping the tray on startup (issue #703).

Changes:
- Add $Script:MinVCRuntimeVersion = 14.40.0.0 constant with explanatory comment
- In the app-local VC runtime check loop, verify vcruntime140.dll file version is
  >= 14.40.0.0 on Windows and emit a clear error pointing to issue #703 when not
- Update script description to document the multi-library VC runtime dependency
- Add ReleaseWorkflow_VCRuntimeMinimumVersionIsEnforced test to pin the check

The version check runs only on Windows (FileVersionInfo reads Windows PE metadata).
CI will catch the mismatch on the next x64/arm64 build leg.

Closes #703

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added automation bug Something isn't working repo-assist labels Jun 7, 2026
@clawsweeper

clawsweeper Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed June 7, 2026, 9:15 AM ET / 13:15 UTC.

Summary
The branch adds a Windows-only minimum VC++ runtime version check to the release native-dependency verifier and a regression test that pins the new gate.

Reproducibility: yes. for a source-level path: current main still pins VCRuntime.CefSharp.140 1.0.5, and the PR adds a required 14.40 Windows verifier without changing that source. I did not run a Windows publish in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Release verifier surface: 1 PowerShell verifier changed. The changed script runs in Windows build and release jobs, so its failure mode can block packaging.
  • Diff size: 2 files changed, +45/-4. The patch is small, but it touches release validation rather than ordinary application code.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Pair the verifier with a 14.40+ app-local runtime source and Windows publish proof.
  • Extend the version gate to cover the VC runtime DLLs the payload actually ships, including msvcp140.dll.

Risk before merge

  • [P1] Merging before updating the x64 app-local runtime source would make Windows build and release validation fail against the current VCRuntime.CefSharp.140 1.0.5 payload instead of producing a fixed release.
  • [P1] The new gate checks only vcruntime140.dll, so a mixed or partially repaired payload could still leave an old msvcp140.dll, which is the module named in the linked crash report.

Maintainer options:

  1. Pair The Gate With Runtime Sourcing (recommended)
    Update or replace the x64 VC++ runtime source in this PR, then prove the Windows build/release verifier passes with 14.40+ DLLs before merge.
  2. Accept A Temporary Fail-Closed Build
    Maintainers could intentionally merge the gate first, but that means owning a broken Windows build/release path until the packaging follow-up lands.
  3. Pause For A Self-Contained Replacement
    If the runtime source choice needs more design, pause or close this draft and replace it with a packaging PR that includes validation in one branch.

Next step before merge

  • [P2] Manual review is needed because the blocker is choosing and validating the permanent x64 app-local VC++ runtime source, not a one-line repair.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes a release verifier script and a test, without adding dependencies, secrets access, or new third-party execution.

Review findings

  • [P1] Update the runtime source before failing the verifier — scripts/Test-ReleaseNativeDependencies.ps1:126
  • [P2] Check the full VC runtime payload — scripts/Test-ReleaseNativeDependencies.ps1:126
Review details

Best possible solution:

Land a self-contained release-runtime fix that sources 14.40+ app-local VC++ DLLs, keeps this verifier as a regression gate, and uses Windows publish proof before closing #703.

Do we have a high-confidence way to reproduce the issue?

Yes for a source-level path: current main still pins VCRuntime.CefSharp.140 1.0.5, and the PR adds a required 14.40 Windows verifier without changing that source. I did not run a Windows publish in this read-only review.

Is this the best way to solve the issue?

No; a verifier is the right kind of regression gate, but this PR is not the best standalone fix because it makes the existing packaging fail without providing a 14.40+ runtime source. The safer path is to pair runtime sourcing and validation in the same PR.

Full review comments:

  • [P1] Update the runtime source before failing the verifier — scripts/Test-ReleaseNativeDependencies.ps1:126
    This PR adds a required 14.40+ check, but src/Directory.Build.props and src/Directory.Build.targets still source the app-local x64 DLLs from VCRuntime.CefSharp.140 1.0.5. The Windows build and release jobs already run this verifier with -RequireAppLocalVCRuntime, so merging this branch by itself turns the current packaging into a failing CI/release path instead of fixing the crash loop. Please update or replace the bundled runtime source in the same PR, or keep the gate out of the required path until that packaging fix lands.
    Confidence: 0.91
  • [P2] Check the full VC runtime payload — scripts/Test-ReleaseNativeDependencies.ps1:126
    The report names MSVCP140.dll 14.29, and this script already enumerates msvcp140*.dll, but the new minimum-version check runs only for an exact vcruntime140.dll name. A mixed or partially repaired payload could still ship an old msvcp140.dll and pass this gate; apply the minimum-version check to the app-local VC runtime DLLs the script collects.
    Confidence: 0.78

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4be005707f44.

Label changes

Label changes:

  • add P0: The PR is the current candidate for a linked startup crash-loop issue that can make the tray unusable on affected installs.
  • add merge-risk: 🚨 automation: The new required verifier can fail current Windows build/release jobs because the app-local runtime source remains VCRuntime.CefSharp.140 1.0.5.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This is a GitHub Actions/Repo Assist bot PR, so the external-contributor real-behavior proof gate does not apply; Windows publish proof is still needed before maintainers merge the release fix.

Label justifications:

  • P0: The PR is the current candidate for a linked startup crash-loop issue that can make the tray unusable on affected installs.
  • merge-risk: 🚨 automation: The new required verifier can fail current Windows build/release jobs because the app-local runtime source remains VCRuntime.CefSharp.140 1.0.5.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This is a GitHub Actions/Repo Assist bot PR, so the external-contributor real-behavior proof gate does not apply; Windows publish proof is still needed before maintainers merge the release fix.
Evidence reviewed

Acceptance criteria:

  • [P1] ./build.ps1.
  • [P1] dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore.
  • [P1] dotnet publish ./src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj -c Release -r win-x64 --self-contained --no-restore -o publish.
  • [P1] pwsh ./scripts/Test-ReleaseNativeDependencies.ps1 -PayloadPath publish -RequireAppLocalVCRuntime.

What I checked:

  • PR diff adds fail-closed runtime version gate: The head version of scripts/Test-ReleaseNativeDependencies.ps1 adds MinVCRuntimeVersion 14.40.0.0 and compares app-local vcruntime140.dll against it when running on Windows. (scripts/Test-ReleaseNativeDependencies.ps1:38, ecde37bc25c4)
  • Current main still uses the known stale x64 runtime package: Current main pins VCRuntime.CefSharp.140 to 1.0.5 in both product package references and the runtime-copy target source. (src/Directory.Build.props:17, 4be005707f44)
  • Current Windows build and release jobs run this verifier as required validation: The CI workflow invokes Test-ReleaseNativeDependencies.ps1 with -RequireAppLocalVCRuntime during the Windows build and again during release packaging. (.github/workflows/ci.yml:394, 4be005707f44)
  • The native stack that needs the newer runtime remains present: Current main references Microsoft.ML.OnnxRuntime 1.26.0 and org.k2fsa.sherpa.onnx 1.13.0, matching the linked crash report and the PR rationale. (src/OpenClaw.Shared/OpenClaw.Shared.csproj:22, 4be005707f44)
  • NuGet package path is not a simple version bump: The NuGet flat-container index currently lists VCRuntime.CefSharp.140 only through 1.0.5, so the PR's suggested package update path needs maintainer packaging choice or a different runtime source.
  • Related issue already identifies this as a crash-loop release-runtime bug: The linked issue discussion reports signed v0.6.3 and current-main repros with app-local msvcp140/vcruntime140 14.29 shadowing the system runtime, and Repo Assist noted follow-up packaging work is still needed.

Likely related people:

  • vincentkoc: Commit 7485ce2 added the VC runtime package path, native dependency verifier, workflow checks, tests, and release docs that this PR modifies. (role: introduced release runtime bundling; confidence: high; commits: 7485ce2af9da; files: src/Directory.Build.props, src/Directory.Build.targets, scripts/Test-ReleaseNativeDependencies.ps1)
  • Keith Mahoney: Commit f839cf5 recently changed adjacent ARM64 VC runtime sourcing, release workflow behavior, docs, and release-signing tests. (role: recent release-runtime contributor; confidence: medium; commits: f839cf53aa0c; files: src/Directory.Build.targets, tests/OpenClaw.Tray.Tests/ReleaseSigningWorkflowTests.cs, docs/RELEASING.md)
  • RBrid: Commit b0ba9af added the Sherpa package reference and Piper OfflineTts wrapper involved in the reported native initialization failure. (role: introduced Sherpa/Piper native dependency; confidence: medium; commits: b0ba9affa25d; files: src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj, src/OpenClaw.Tray.WinUI/Services/TextToSpeech/PiperTextToSpeechClient.cs, src/OpenClaw.Shared/Audio/PiperVoiceManager.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 7, 2026
@shanselman

Copy link
Copy Markdown
Collaborator

superceded by #713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation bug Something isn't working rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. repo-assist status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tray crash-loops on startup — bundled vc++ runtime older than onnxruntime needs

1 participant