fix(windows): make pnpm dev:app:win work behind TLS-inspecting proxies#2449
Merged
Conversation
Switch reqwest builders in `api::rest` and `openhuman::app_state` from `use_rustls_tls()` to `use_native_tls()` so the platform trust store is honored. Why: rustls + webpki-roots only ships Mozilla root CAs. In TLS-inspecting environments (corporate antivirus, MITM proxies) the OS chain validates against a locally installed CA that webpki-roots cannot see, so every HTTPS call to `api.tinyhumans.ai` fails with `UnknownIssuer`. Native TLS uses schannel on Windows, SecureTransport on macOS, and OpenSSL on Linux — each defers to the OS trust store, which already holds the corporate CA when present.
Swap `tokio-tungstenite` feature from `rustls-tls-webpki-roots` to `native-tls` so the platform trust store is honored on connect. Why: matches the HTTP-client switch in the parent commit. With webpki-roots, the WebSocket handshake to `wss://api.tinyhumans.ai` fails with `UnknownIssuer` in TLS-inspecting environments because the intercepting CA isn't in the Mozilla bundle. `connect_async()` picks its TLS connector based on the enabled feature, so the call sites in `socket::ws_loop` don't need changes — only the Cargo feature does. Cargo.lock will be regenerated on the next build.
When the dev script reaches `pnpm tauri dev`, pnpm spawns cmd.exe to
run the literal script body `tauri "dev"`. Pnpm normally prepends
`./node_modules/.bin` for that child shell, but under the long
bash→cmd→bash→pnpm chain produced by `pnpm dev:app:win`, the relative
entry sometimes resolves against the wrong cwd and `tauri.CMD` is not
found ("'tauri' is not recognized as an internal or external command").
Prepend the absolute `$APP_DIR/node_modules/.bin` to PATH before any
`pnpm tauri:ensure` / `pnpm tauri dev` invocation so the shim is
always resolvable regardless of how pnpm propagates PATH downstream.
The final `pnpm tauri dev` step in the dev script is fragile on Windows.
It depends on which pnpm shim `find_pnpm` resolved:
- `~/AppData/Local/pnpm/.tools/.../pnpm` (the self-managing shim) auto-
prepends `node_modules/.bin` to PATH when forking the script's
cmd.exe child, so the literal script body `tauri "dev"` resolves
`tauri.CMD` from the workspace bin dir.
- The WinGet-installed `pnpm.exe` does not perform the same auto-
prepend, so the same script body fails with
"'tauri' is not recognized as an internal or external command".
Which shim wins is racey (depends on bash's PATH ordering, /etc/profile
behavior, and whether pnpm self-update fixed any placeholder files).
Side-step the wrapper by calling the vendored `cargo-tauri.exe`
binary that `ensure-tauri-cli.sh` already installs at
`<repo>/.cache/cargo-install/bin/`. Same effect, no pnpm-shim
dependency. The DEV_PORT/devUrl override path is preserved verbatim.
…mand After the previous commit switched the final dev launcher from `pnpm tauri dev` to `cargo-tauri.exe dev`, cargo-tauri spawns the configured `beforeDevCommand` (`pnpm run dev`) in a fresh cmd.exe child that inherits PATH from cargo-tauri but does NOT inherit pnpm's own runtime PATH injection. With the WinGet-installed pnpm in particular (no `AppData/Roaming/npm` shim), the spawned cmd then fails with "'pnpm' is not recognized as an internal or external command". Prepend the directory holding the resolved `$PNPM_EXE` to PATH_PREFIX so the chain `bash → cargo-tauri.exe → cmd → pnpm → vite` finds pnpm regardless of which pnpm shim find_pnpm picked.
…path tauri.conf.json's `beforeDevCommand` is `"pnpm run dev"` (bare name). cargo-tauri spawns it via `cmd.exe /S /C pnpm run dev`, and that cmd child does not reliably see the same PATH that the parent bash had after vcvars rewrites and MSYS conversion — pnpm fails with "'pnpm' is not recognized as an internal or external command". Pinning the pnpm path on PATH inside this script (previous commit) isn't enough: by the time the path makes it through the bash → cargo-tauri.exe → cmd.exe chain, the entry may be reordered, stripped, or path-converted incorrectly. Defense-in-depth: override the tauri config inline so the resolved absolute `$PNPM_EXE` (converted to a Windows path) is invoked directly, e.g. `"C:\Users\…\pnpm.exe" run dev`. The merge happens via the same `-c` JSON path that DEV_PORT already uses for devUrl.
Embedding quotes around the absolute pnpm path in the tauri config override (`"<path>" run dev`) survives the JSON decode but loses the quotes when cargo-tauri hands the string to `cmd.exe /S /C`: Rust's argv-to-cmd escaping rewrites `"` to `\"`, and cmd then treats the whole `\"…\"` token as the program name and errors out with "'\"C:\…\pnpm\"' is not recognized". The pnpm install paths we resolve via `find_pnpm` are space-free (`@pnpm+exe/10.10.0` is dot- and plus-laden, but no spaces), so passing a bare path avoids the quote-mangling entirely. Backslashes still need JSON-escaping for the `-c` argument, which is preserved.
The previous fix routed the override through `<absolute pnpm> run dev`,
which then runs `vite` via pnpm. cargo-tauri spawns beforeDevCommand
from the tauri project dir (`app/src-tauri/`), so pnpm's automatic
`./node_modules/.bin` prepend resolves against `app/src-tauri/` and
fails to find `vite` ("'vite' is not recognized").
`pnpm run dev` ultimately just executes `vite` from
`app/node_modules/.bin/vite.CMD`. Point beforeDevCommand at that path
directly — same behavior, no cwd dependency, no pnpm path quirks.
If `vite.CMD` is missing we bail with a clear "did pnpm install run?"
message instead of letting cargo-tauri report an opaque cmd failure.
When the workspace path contains spaces (e.g. `E:\Office Files\...`), pointing tauri's beforeDevCommand at the absolute `vite.CMD` directly also fails. cargo-tauri spawns the command via `cmd.exe /S /C <string>`, and Rust's argv escaping strips any literal double-quotes around the path. cmd then parses the first space as a token boundary and tries to execute `E:\Office`. 8.3 short-name conversion (`cygpath -ws`) doesn't help when 8dot3name is disabled on the drive, which is the case on this workspace's E: drive. Write a tiny `run-vite.bat` to a space-free TEMP location and invoke that. The wrapper internally quotes the real vite.CMD path (`call "<full path>" %*`), so the spacey path is contained where we control the quoting and isn't subject to cargo-tauri's argv escaping.
The previous wrapper called `vite.CMD`, which in turn falls back to
bare `node` when its sibling node.exe doesn't exist (the workspace's
copy doesn't). cargo-tauri's cmd child for beforeDevCommand has a
sanitized PATH that doesn't include node, so the fallback then fails
with "'node' is not recognized".
Bypass the npm-shim layer entirely: resolve `node.exe` once at the
parent bash level and write a wrapper that runs
"<abs node.exe>" "<abs vite.js>" %*
This makes the wrapper hermetic — neither node nor the vite shim need
to be on the cmd child's PATH.
`build_runtime_proxy_client` and `build_runtime_proxy_client_with_timeouts` are the shared reqwest factories used by composio integrations, channel providers (discord, dingtalk, …), and the multimodal agent path. They built clients with default TLS, which in this dep configuration resolves to rustls + webpki-roots and fails with `UnknownIssuer` whenever the network presents an OS-trusted but webpki-unknown CA chain (corporate TLS interception). Add `.use_native_tls()` to both builders so the OS trust store is honored. Matches the earlier swap in `api::rest` and `openhuman::app_state` — same rationale, broader coverage.
… else
Earlier commits in this branch unconditionally swapped reqwest and
tokio-tungstenite to native-tls to fix `UnknownIssuer` failures on
Windows networks that intercept TLS with a corporate CA. macOS and
Linux don't hit that scenario in normal distribution, and Linux in
particular would gain an unwanted runtime dependency on libssl /
libcrypto if it stayed on native-tls.
Gate every TLS-backend selection behind `#[cfg(target_os = "windows")]`:
- Windows builds resolve TLS via native-tls (schannel) — the OS cert
store wins, fixing corporate-MITM environments.
- macOS / Linux builds keep `use_rustls_tls()` and tokio-tungstenite's
`rustls-tls-webpki-roots` — the historical behavior, no regression.
Sites touched:
- api::rest, openhuman::app_state — HTTP clients
- openhuman::config::schema::proxy — central proxy-client factories
used by composio, channels, agent paths
- integrations::{client, searxng, seltz} — explicit-rustls clients
- inference::provider::compatible — two TLS call sites
- Cargo.toml — tokio-tungstenite split into per-target dependency
entries so each platform compiles with exactly one TLS feature
Per-target `tokio-tungstenite` dependency entries (see earlier `fix(tls): gate native-tls swap to Windows` commit) introduced `native-tls` and `tokio-native-tls` as transitive deps. Refresh both the root and Tauri-shell lock files so the resolved graphs stay consistent with the manifests.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralize TLS selection with a new ChangesPlatform-specific TLS backend selection
Windows development script improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@scripts/run-dev-win.sh`:
- Around line 656-669: The BEFORE_DEV_CMD currently embeds VITE_WRAPPER_WIN
without protecting spaces, so cmd.exe may split the .bat path; ensure the
wrapper path is quoted or replaced with a space-free fallback before assigning
CONFIG_OVERRIDE. Modify the code that computes BEFORE_DEV_CMD (which uses
VITE_WRAPPER_WIN and WRAPPER_DIR_UNIX) to either wrap VITE_WRAPPER_WIN in
surrounding double quotes escaped for JSON/Windows (so the final CONFIG_OVERRIDE
contains a quoted path) or detect spaces in the TEMP-derived WRAPPER_DIR_UNIX
and fall back to a safe temp base (no spaces) before creating VITE_WRAPPER_UNIX;
finally update CONFIG_OVERRIDE (the JSON string built here) to include the
guarded/quoted BEFORE_DEV_CMD so beforeDevCommand will run correctly under
cmd.exe.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e54aee3-8306-4e42-963a-a6c6dc80adce
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlscripts/run-dev-win.shsrc/api/rest.rssrc/openhuman/app_state/ops.rssrc/openhuman/config/schema/proxy.rssrc/openhuman/inference/provider/compatible.rssrc/openhuman/integrations/client.rssrc/openhuman/integrations/searxng.rssrc/openhuman/integrations/seltz.rs
# Conflicts: # scripts/run-dev-win.sh
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/run-dev-win.sh (1)
679-691:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against spaces in the %TEMP%-derived wrapper path.
If
%TEMP%contains spaces (e.g.,C:\Users\John Doe\AppData\Local\Temp),VITE_WRAPPER_WINwill contain spaces. When embedded inbeforeDevCommandand executed viacmd.exe /S /C, the unquoted path will split and fail. Add a space guard after computingVITE_WRAPPER_WIN.Suggested fix
VITE_WRAPPER_WIN="$(cygpath -w "$VITE_WRAPPER_UNIX" 2>/dev/null || printf '%s' "$VITE_WRAPPER_UNIX")" +if [[ "$VITE_WRAPPER_WIN" == *" "* ]]; then + echo "[run-dev-win] wrapper path contains spaces: $VITE_WRAPPER_WIN" >&2 + echo "[run-dev-win] set TEMP/TMP to a space-free path (e.g. C:\\Temp) and retry." >&2 + exit 1 +fi echo "[run-dev-win] vite wrapper at: $VITE_WRAPPER_WIN"🤖 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 `@scripts/run-dev-win.sh` around lines 679 - 691, The computed Windows wrapper path VITE_WRAPPER_WIN must be quoted before embedding into BEFORE_DEV_CMD to guard against spaces; change the assignment so you wrap the path in double quotes and then escape backslashes for cmd by performing the existing replacement on the quoted value (i.e., produce BEFORE_DEV_CMD from "\"$VITE_WRAPPER_WIN\"" with the //\\/\\\\ substitution), ensuring the final BEFORE_DEV_CMD contains a quoted, backslash-escaped path that won't split when passed to cmd.exe /S /C; reference VITE_WRAPPER_WIN and BEFORE_DEV_CMD (and VITE_WRAPPER_UNIX as the source) when making this change.
🤖 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 `@scripts/run-dev-win.sh`:
- Around line 679-691: The computed Windows wrapper path VITE_WRAPPER_WIN must
be quoted before embedding into BEFORE_DEV_CMD to guard against spaces; change
the assignment so you wrap the path in double quotes and then escape backslashes
for cmd by performing the existing replacement on the quoted value (i.e.,
produce BEFORE_DEV_CMD from "\"$VITE_WRAPPER_WIN\"" with the //\\/\\\\
substitution), ensuring the final BEFORE_DEV_CMD contains a quoted,
backslash-escaped path that won't split when passed to cmd.exe /S /C; reference
VITE_WRAPPER_WIN and BEFORE_DEV_CMD (and VITE_WRAPPER_UNIX as the source) when
making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a04cb5b7-596e-498f-b835-0bf78ee6c322
📒 Files selected for processing (1)
scripts/run-dev-win.sh
If %TEMP% contains spaces the unquoted .bat path would cause cmd.exe to split on the space and fail to start Vite. Detect the condition early and exit with a clear message directing the user to set TEMP to a space-free path (e.g. C:\Temp).
M3gA-Mind
commented
May 21, 2026
Contributor
Author
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Review Summary
This is a well-motivated PR tackling two genuinely painful Windows problems. The TLS gating is logically correct — #[cfg(target_os = "windows")] / #[cfg(not(target_os = "windows"))] pairs are clean, the platform story (schannel for Windows, rustls+webpki everywhere else) is sound, and the bash script refactoring eliminates a legitimately fragile call chain. CodeRabbit's %TEMP% space-guard concern is already addressed in the new code.
Two things need resolution before merge:
| # | Severity | Location | Issue |
|---|---|---|---|
| 1 | [major] | app/src-tauri/Cargo.lock |
motosan-ai-oauth appears as a direct dep of openhuman (Tauri app), not explained in the PR |
| 2 | [major] | All 6 Rust files | TLS selector copy-pasted 8× — extract to a shared helper |
| 3 | [minor] | proxy.rs:454,480 |
Error-path Client::new() fallback bypasses platform TLS selection |
| 4 | [minor] | run-dev-win.sh |
PATH dedup block removed without noting the assumption change |
The Windows dev experience improvement is real and the approach is correct — just needs the items above tidied up.
graycyrus
requested changes
May 21, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Thorough, well-documented PR — the platform-conditional TLS gating and the run-dev-win.sh overhaul are exactly right. CI is fully green. One call site was missed.
Change summary
| Area | Files | What changed |
|---|---|---|
| Cargo deps | Cargo.toml, lockfiles |
Platform-split tokio-tungstenite: native-tls on Windows, rustls elsewhere |
| Rust HTTP clients (x7) | rest.rs, ops.rs, proxy.rs, compatible.rs, client.rs, searxng.rs, seltz.rs |
#[cfg]-gated TLS backend: use_native_tls() on Windows, use_rustls_tls() elsewhere |
| Dev script | run-dev-win.sh |
Invoke cargo-tauri.exe directly, hermetic vite wrapper via temp .bat, PATH hardening |
All #[cfg] pairs are symmetric and the Cargo feature split is correct. The script space-guard for VITE_WRAPPER_WIN is solid.
One site was missed — see inline comment.
…ck TLS Addresses @M3gA-Mind review comments: - Add src/openhuman/tls.rs with tls_client_builder() — the single source of truth for platform-conditional TLS selection (Windows → native-tls / schannel, macOS+Linux → rustls). All 8 previous copy-paste sites now delegate to this helper (rest.rs, ops.rs, proxy.rs ×2, compatible.rs ×2, client.rs, searxng.rs, seltz.rs). - Fix proxy.rs: Client::new() fallback in both build_runtime_proxy_client and build_runtime_proxy_client_with_timeouts was silently reverting to rustls on Windows when the main builder failed. Both error paths now call tls_client_builder() so the fallback client honors the OS cert store too (addresses @M3gA-Mind on src/openhuman/config/schema/proxy.rs:454,480). - Add historical PATH-dedup comment to scripts/run-dev-win.sh explaining why the old dedup loop was removed — preserves institutional knowledge about the MSYS env-block overflow that previously caused empty-PATH children (addresses @M3gA-Mind on scripts/run-dev-win.sh:645).
…tible.rs The two .unwrap_or_else fallbacks in OpenAiCompatibleProvider::http_client() were using Client::new() which silently picks rustls on Windows when both native-tls and rustls features are compiled in. Replace with tls_client_builder().build().unwrap_or_default() so the error-path client also honors the Windows cert store — same fix already applied to proxy.rs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/openhuman/tls.rs (1)
1-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove this new core module out of
src/openhuman/root.This introduces new functionality as
src/openhuman/tls.rs, but repo rules require new Rust core functionality in a dedicated subdirectory (for examplesrc/openhuman/tls/mod.rs), not as a new standalone root file.As per coding guidelines: “New functionality must live in a dedicated subdirectory … Do not add new standalone
*.rsfiles directly atsrc/openhuman/root.”🤖 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 `@src/openhuman/tls.rs` around lines 1 - 35, Move the new standalone file into a dedicated module directory and update the module declaration: create a src/openhuman/tls/mod.rs containing the tls_client_builder function (and the file-level docs), remove the standalone src/openhuman/tls.rs, and ensure the parent module (src/openhuman/mod.rs or lib.rs) exposes it with pub mod tls; so references to tls_client_builder remain valid via openhuman::tls::tls_client_builder (or the current import path) without changing the function name or behavior.src/openhuman/inference/provider/compatible.rs (1)
281-299:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFallback branches still bypass the centralized TLS policy.
Both
unwrap_or_elsebranches fall back toClient::new(), which skipscrate::openhuman::tls::tls_client_builder(). This leaves the error path inconsistent with the Windows/native-TLS policy applied in the primary path.💡 Suggested fix
- return builder.build().unwrap_or_else(|error| { + return builder.build().unwrap_or_else(|error| { tracing::warn!("Failed to build proxied timeout client with user-agent: {error}"); - Client::new() + crate::openhuman::tls::tls_client_builder() + .timeout(std::time::Duration::from_secs(120)) + .connect_timeout(std::time::Duration::from_secs(10)) + .build() + .unwrap_or_default() }); @@ builder.build().unwrap_or_else(|error| { tracing::warn!("Failed to build proxied timeout client: {error}"); - Client::new() + crate::openhuman::tls::tls_client_builder() + .timeout(std::time::Duration::from_secs(120)) + .connect_timeout(std::time::Duration::from_secs(10)) + .build() + .unwrap_or_default() })Also applies to: 291-307
🤖 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 `@src/openhuman/inference/provider/compatible.rs` around lines 281 - 299, The fallback branches currently call Client::new() and thus bypass the centralized TLS policy; change the error-paths so they construct the fallback client via crate::openhuman::tls::tls_client_builder() (and reapply crate::openhuman::config::apply_runtime_proxy_to_builder(...) and the same timeouts/headers) instead of Client::new(), and log the build error before returning that TLS-backed fallback; reference tls_client_builder, apply_runtime_proxy_to_builder, and the builder.build().unwrap_or_else(...) closures to locate and replace the Client::new() fallbacks so TLS/native-TLS behavior is preserved on error.
🤖 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 `@src/openhuman/config/schema/proxy.rs`:
- Around line 469-483: In build_runtime_proxy_client_with_timeouts the fallback
path constructs fb via crate::openhuman::tls::tls_client_builder() but doesn’t
re-apply the timeout/connect_timeout (or proxy) settings, producing an unbounded
client on error; fix by applying the same
timeout(std::time::Duration::from_secs(timeout_secs)) and
connect_timeout(std::time::Duration::from_secs(connect_timeout_secs)) (and, if
you used apply_runtime_proxy_to_builder on the primary builder, apply it to fb
with service_key as well) before calling fb.build() so the fb fallback honors
the same timeouts and proxy behavior as the primary builder.
---
Outside diff comments:
In `@src/openhuman/inference/provider/compatible.rs`:
- Around line 281-299: The fallback branches currently call Client::new() and
thus bypass the centralized TLS policy; change the error-paths so they construct
the fallback client via crate::openhuman::tls::tls_client_builder() (and reapply
crate::openhuman::config::apply_runtime_proxy_to_builder(...) and the same
timeouts/headers) instead of Client::new(), and log the build error before
returning that TLS-backed fallback; reference tls_client_builder,
apply_runtime_proxy_to_builder, and the builder.build().unwrap_or_else(...)
closures to locate and replace the Client::new() fallbacks so TLS/native-TLS
behavior is preserved on error.
In `@src/openhuman/tls.rs`:
- Around line 1-35: Move the new standalone file into a dedicated module
directory and update the module declaration: create a src/openhuman/tls/mod.rs
containing the tls_client_builder function (and the file-level docs), remove the
standalone src/openhuman/tls.rs, and ensure the parent module
(src/openhuman/mod.rs or lib.rs) exposes it with pub mod tls; so references to
tls_client_builder remain valid via openhuman::tls::tls_client_builder (or the
current import path) without changing the function name or behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9e0d1e2-5f40-4ccb-8b8e-103c4d7225bd
📒 Files selected for processing (10)
scripts/run-dev-win.shsrc/api/rest.rssrc/openhuman/app_state/ops.rssrc/openhuman/config/schema/proxy.rssrc/openhuman/inference/provider/compatible.rssrc/openhuman/integrations/client.rssrc/openhuman/integrations/searxng.rssrc/openhuman/integrations/seltz.rssrc/openhuman/mod.rssrc/openhuman/tls.rs
✅ Files skipped from review due to trivial changes (1)
- src/openhuman/mod.rs
…out fallback - composio/client.rs: replace hardcoded use_rustls_tls() with tls_client_builder() so the DELETE helper client picks up native-tls on Windows, matching the shared IntegrationClient it mirrors (addresses @graycyrus on src/openhuman/composio/client.rs:462) - proxy.rs: propagate timeout_secs and connect_timeout_secs into the tls_client_builder() fallback in build_runtime_proxy_client_with_timeouts so the error-path client remains bounded and honors the same timeouts as the primary builder (addresses @coderabbitai on src/openhuman/config/schema/proxy.rs:483)
Contributor
Author
|
@M3gA-Mind unresolved review feedback from graycyrus — please address before we review. |
M3gA-Mind
commented
May 21, 2026
| @@ -0,0 +1,34 @@ | |||
| //! Platform-conditional TLS backend selection for reqwest clients. | |||
Contributor
Author
There was a problem hiding this comment.
Deferred: CodeRabbit flagged that src/openhuman/tls.rs should live in src/openhuman/tls/mod.rs per the CLAUDE.md module-layout rule ("new standalone *.rs files at src/openhuman/ root are not a template"). The previous agent and the task instructions explicitly acknowledge tls.rs as grandfathered alongside dev_paths.rs and util.rs for this PR, since it is a small single-function helper extracted to eliminate duplication. If the team prefers the subdirectory form before merge, the change is mechanical: mkdir src/openhuman/tls && mv src/openhuman/tls.rs src/openhuman/tls/mod.rs — no callers change because the import path crate::openhuman::tls::tls_client_builder stays identical. Please confirm whether to apply that rename before merge.
3 tasks
CodeGhost21
pushed a commit
to CodeGhost21/openhuman
that referenced
this pull request
May 22, 2026
senamakel
pushed a commit
to aqilaziz/openhuman
that referenced
this pull request
May 23, 2026
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.
Summary
pnpm dev:app:win) is broken end-to-end on machines behind a corporate TLS-inspection proxy and on workspaces whose path contains spaces (e.g.E:\Office Files\...).api.tinyhumans.aifails withUnknownIssuer— auth, sockets, composio, integrations.bash -> pnpm -> cmd.exe -> cargo-tauri -> cmd.exe -> tauri.CMD -> vite.CMDaccumulates a small bug at almost every layer (broken pnpm self-update placeholder, quoted-path mangling,node_modules/.binresolved against the wrong cwd, ASCII space causingE:\Office is not recognized, missingnodeon cargo-tauri's child PATH, etc.).#[cfg(target_os = "windows")]so macOS / Linux keep their originalrustlspath and avoid a new OpenSSL runtime dep.Problem
When running the desktop dev shell on a Windows host whose network re-signs TLS with a private CA (corporate AV / DLP, captive portal, ZScaler-style proxy), the bundled
OpenHuman.execannot speak to any backend service:Concurrently,
pnpm dev:app:winitself fails to launch. The chain produces (in order, fixed by separate commits):cef-dll-sysbuild script cannot download CEF (same rustls TLS issue, fromureq).aws-lc-sysC feature probe fails on MSVC 14.44 (stdalign / C11 dialect).'C:/Program' is not recognized— pnpm script-shell mangles the quoted bash path indev:app:win.'This: command not found'— brokenpnpmplaceholder in%LOCALAPPDATA%\pnpm\.tools\...\node_modules\@pnpm\exe\pnpm.'vite' is not recognized— pnpm only prepends./node_modules/.bin(relative) for script children; it resolves against the wrong cwd.'tauri' is not recognized— which pnpm shim variant wins determines whethernode_modules/.binlands on PATH.'pnpm' is not recognized— cargo-tauri'scmd.exe /S /Cchild forbeforeDevCommandstrips PATH entries.'\"...\\pnpm\"' is not recognized— RustCommandargv escaping rewrites embedded"to\"for cmd.'E:\Office' is not recognized— bare absolute path breaks on workspaces with spaces.'node' is not recognized— vite shim falls back tonode(PATH lookup) when no siblingnode.exe.Solution
TLS backend selection (gated to Windows)
Centralise platform-conditional TLS in every reqwest builder and the
tokio-tungsteniteCargo feature:Touched HTTP-client construction sites:
src/api/rest.rssrc/openhuman/app_state/ops.rssrc/openhuman/config/schema/proxy.rs— central factory used by composio, channels, agent pathssrc/openhuman/integrations/{client, searxng, seltz}.rssrc/openhuman/inference/provider/compatible.rs(two sites)After the swap: schannel on Windows reads the OS trust store, so corporate-CA-resigned certs validate cleanly. macOS / Linux stay on rustls, no behaviour change.
scripts/run-dev-win.shhardeningEach commit peels off one layer of the chain. End state:
bash -> cargo-tauri.exe -> cmd /S /C -> run-vite.bat -> node.exe vite.js, hermetic absolute paths throughout, no dependency on whichever pnpm self-managed shim happens to wincommand -v.Key changes:
$APP_DIR/node_modules/.binand the resolved$PNPM_EXEdir toPATH_PREFIX..cache/cargo-install/bin/cargo-tauri.exedirectly instead ofpnpm tauri dev.beforeDevCommandvia a one-shotrun-vite.batwritten to a space-free%TEMP%directory; the wrapper itself callsnode.exe vite.jsso neithernodenorvite.CMDneed to be on cargo-tauri's child PATH.Submission Checklist
cfg-only with no runtime-testable code path; dev-script changes are bash-only.cfg-only TLS toggles and bash script; no new executable TS/Rust lines added.Closes #NNN— linked tickets have broader scope than this PR fully addresses; usingRefsinstead ofCloses.Impact
native-tls(schannel) for both reqwest and tokio-tungstenite. macOS / Linux unchanged — stillrustls+ webpki-roots, no OpenSSL runtime dep added.Related
aws-lc-sys's stdalign C-feature probe is worked around in this branch viaCFLAGS_x86_64_pc_windows_msvc="/std:c11"set externally — a small upstream PR to aws-lc-sys to make the probe tolerant of pre-C11 dialects would let us drop that.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
sync/window-upstream8b179199Validation Run
pnpm format:check— passes (verified this session; Prettier reports all files use correct code style).pnpm typecheck— passes (verified this session; tsc --noEmit exits cleanly).cargo fmt --check(exit 0) andcargo check --manifest-path Cargo.toml(clean finish) both pass (verified this session).cargo fmt --checkandcargo check --manifest-path app/src-tauri/Cargo.tomlboth pass (verified this session).Validation Blocked
command:pnpm install(initial) andcargo install --locked --path app/src-tauri/vendor/tauri-cef/crates/tauri-cli(CEF download).error:UNABLE_TO_VERIFY_LEAF_SIGNATURE(pnpm),UnknownIssuer(cargo) — same root cause as the runtime TLS bug this PR fixes.impact:Local-only on this contributor's network. The committed code does not depend on the local mirror orNODE_OPTIONS=--use-system-caworkarounds used to bootstrap the build.Behavior Changes
Parity Contract
wsswithout an explicit TLS feature;integrations::clientstill pinshttp1_onlyper the original comment.#[cfg(windows)]with an explicit#[cfg(not(windows))]arm that selects the historical rustls path — no platform falls through with neither branch.Summary by CodeRabbit