fix(windows): silence conhost flashes from core-side spawns (#731/#1338 follow-up)#1498
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughSuppresses console windows on Windows for spawned subprocesses by adding a cross-platform helper and applying CREATE_NO_WINDOW across service runners, local-AI probes/installers, diagnostics, node runtime probes, and Windows task queries. ChangesWindows Process No-Window Suppression
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
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 `@src/openhuman/doctor/core.rs`:
- Around line 745-746: The match expression `match child_cmd.output()` in
core.rs is split across two lines causing cargo fmt failure; collapse it so the
opening brace sits on the same line as the match (i.e., `match
child_cmd.output() {`) and then run `cargo fmt` (or `cargo fmt --all` / your
editor formatter) to ensure the rest of the surrounding block (the arms handling
the Result) is properly formatted and passes `cargo fmt --check`.
🪄 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: e289d401-30ea-4c3f-bdad-8de5393a2e52
📒 Files selected for processing (9)
src/openhuman/doctor/core.rssrc/openhuman/local_ai/device.rssrc/openhuman/local_ai/install.rssrc/openhuman/local_ai/mod.rssrc/openhuman/local_ai/process_util.rssrc/openhuman/local_ai/service/ollama_admin.rssrc/openhuman/node_runtime/resolver.rssrc/openhuman/service/common.rssrc/openhuman/service/windows.rs
/tinyhumansai#1338 follow-up) On Windows, `CreateProcess` allocates a conhost for every child unless `CREATE_NO_WINDOW` (creation flag `0x0800_0000`) is set. The shell-side spawns were fixed in tinyhumansai#731 (core sidecar) and tinyhumansai#1338 (netstat/taskkill). The core-side spawns were not — so any frontend-polled RPC that fans out into a native command (e.g. `local_ai_device_profile` -> `nvidia-smi`) flashes a console window on every poll. On a fresh Windows install with no Ollama, the combined effect was a continuous terminal-flash storm before the auth screen had even rendered. Sites covered: - `local_ai/install.rs`: PowerShell wrapper that runs OllamaSetup.exe - `local_ai/service/ollama_admin.rs`: `ollama --version`, `ollama serve`, candidate probe in `command_works`, `taskkill /F /IM ollama.exe` - `local_ai/device.rs`: `nvidia-smi --query-gpu` — re-probed by `handle_local_ai_device_profile` on every poll - `doctor/core.rs`: PowerShell `Get-PSDrive` for disk space, and `<cmd> --version` for `git` / `curl` / etc. - `node_runtime/resolver.rs`: `<bin> --version` in `probe_subcommand_version` - `service/common.rs` helpers (`run_checked`, `run_capture`, `run_best_effort`, `run_check_silent`) — silences every Windows `schtasks` call at the helper boundary (covers `status`, `start`, `stop`, `install`, `uninstall`, `is_task_exists_windows`). Linux `systemctl` and macOS `launchctl` callers go through the same helpers; `no_window` is a `cfg(not(windows))` no-op for them. New helper `local_ai/process_util::apply_no_window` keeps the `cfg(windows)` + `creation_flags` detail out of call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse `match child_cmd.output()\n {` onto a single line as
required by rustfmt. Fixes cargo fmt --check CI failure on this branch.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
22ff225 to
84c24b7
Compare
…nsai#731/tinyhumansai#1338 follow-up) (tinyhumansai#1498) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
CREATE_NO_WINDOW(creation_flags(0x0800_0000)) to every core-side child-process spawn on Windows.CreateProcess, causing a continuous terminal-flash storm when polled RPCs fan out into native command probes (e.g.local_ai_device_profile→nvidia-smi).src/openhuman/local_ai/process_util.rsexposesapply_no_window(&mut tokio::process::Command)so call sites don't repeat thecfg(windows)block.Problem
On a fresh Windows install, any frontend-polled RPC that fans out into a native command probe (e.g.
local_ai_device_profile→nvidia-smi --query-gpu) caused a continuous conhost flash storm — visible before the auth screen had even rendered.windows_subsystem = "windows"on the parent process (Tauri shell) prevents that process from inheriting a console, but eachCreateProcesscall for a child still allocates a new conhost unlessCREATE_NO_WINDOWis explicitly passed indwCreationFlags. #731 fixed the core sidecar spawn; #1338 fixednetstat/taskkillin the shell. The core-side spawns were missed in both passes.Solution
local_ai/process_util::apply_no_windowwraps the#[cfg(windows)]+creation_flags(0x0800_0000)detail; callers just call the helper on theirCommand.std::process::Commandsites and one-off cases, thecfgblock is inlined directly.local_ai/install.rs: PowerShell wrapper runningOllamaSetup.exelocal_ai/service/ollama_admin.rs:ollama --version,ollama serve,command_workscandidate probe,taskkill /F /IM ollama.exelocal_ai/device.rs:nvidia-smi --query-gpu— re-probed byhandle_local_ai_device_profileon every poll (the main visible loop)doctor/core.rs: PowerShellGet-PSDrivefor disk space +<cmd> --versionforgit/curl/etc. incheck_command_availablenode_runtime/resolver.rs:<bin> --versioninprobe_subcommand_versionservice/common.rshelpers (run_checked,run_capture,run_best_effort,run_check_silent) — silences everyschtaskscall at the helper boundary (covers status/start/stop/install/uninstall). Linuxsystemctland macOSlaunchctlgo through the same helpers;no_windowis acfg(not(windows))no-op on those targets.service/windows.rs:is_task_exists_windows: bypasses the common helpers; patched directly withcreation_flags.Submission Checklist
process_util.rscarries a smoke test assertingapply_no_windowis callable on every platform. Assertable behavior beyond that requires inspecting Windows process-creation flags at the OS level, which is not reachable from Rust unit tests.## Related— no feature-matrix entries touched.windows_subsystem="windows"linker attribute on the parent.Closes #NNN— the core-side conhost flash bug was not tracked under its own issue; see## Relatedfor prior-art references.Impact
Windows-only fix. On Windows, every core-side child-process spawn now sets
CREATE_NO_WINDOW; no conhost is allocated. No behavior change on macOS or Linux — thecfg(not(windows))path is a no-op. No UI surface change, no new API, no migration needed.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/windows-conhost-flashes61c724be51046ece40618811121c6b46791f4c60Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckprocess_util.rssmoke test (callable on all platforms)cargo check -p openhuman --target x86_64-pc-windows-msvcclean (only pre-existing warnings inslack_backfill.rs);cargo fmt --checkcleanValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
CREATE_NO_WINDOW; no conhost window appears for any native probe.Parity Contract
apply_no_windowand inlinecfgblocks are no-ops on non-Windows targets.service/common.rshelpers used by Linuxsystemctland macOSlaunchctlare unaffected;creation_flagspath is Windows-only.Duplicate / Superseded PR Handling
Summary by CodeRabbit