Fall back to CIM when wmic is unavailable - #5305
Conversation
enumerate_all_processes shells out to wmic, which Microsoft removed in Windows 11 24H2. On those builds the spawn fails, enumerate_all_processes returns Err, and startup recovery sees no processes at all. Tries wmic first so older builds keep the existing path, then falls back to Get-CimInstance. The fallback emits the same Key=Value blocks, so parse_wmic_list_output handles both sources unchanged and no parsing code is added. Confirmed on Windows 11 that wmic is absent and that the CIM command returns Caption, CommandLine, ExecutablePath, ParentProcessId and ProcessId in the expected format.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWindows process enumeration tries WMIC first. If WMIC fails or returns no processes, it uses PowerShell CIM. CIM produces UTF-8, WMIC-compatible records for the existing parser. Tests cover fallback selection, failures, encoding, fields, and parsing edge cases. ChangesProcess enumeration
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant enumerate_all_processes
participant WMIC
participant PowerShell_CIM
participant WMIC_list_parser
enumerate_all_processes->>WMIC: enumerate processes
WMIC-->>enumerate_all_processes: records, empty output, or error
enumerate_all_processes->>PowerShell_CIM: enumerate when WMIC fails or is empty
PowerShell_CIM-->>WMIC_list_parser: WMIC-compatible UTF-8 records
WMIC_list_parser-->>enumerate_all_processes: parsed ProcessInfo values
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
|---|---|
| app/src-tauri/src/process_recovery.rs | Adds select_enumeration dispatch layer with enumerate_via_wmic / enumerate_via_cim fallback; includes logging on every branch, UTF-8 pin in the PowerShell script, and full unit-test coverage for the new selection logic and the parser against non-ASCII CIM output. No logic errors found. |
Reviews (4): Last reviewed commit: "test(windows): use a genuinely non-ASCII..." | 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: 3
🤖 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 `@app/src-tauri/src/process_recovery.rs`:
- Around line 760-769: Extract the WMIC/CIM fallback selection and command
construction from enumerate_via_wmic into pure or injectable helpers, then add
Rust tests covering non-empty WMIC short-circuiting, WMIC failure or empty
output falling back to CIM, CIM failure propagation, empty CIM output, and
serialization/parsing of comma-containing command lines and paths, null fields,
and non-ASCII text without spawning system commands.
- Around line 760-769: Update enumerate_via_cim to pass -ErrorAction Stop to
Get-CimInstance and return an error instead of Ok when parsing produces an empty
process list; preserve successful non-empty snapshots and the existing fallback
behavior in the caller.
- Around line 807-811: Update the Windows enumeration helpers
enumerate_via_wmic() and enumerate_via_cim() to use a shared timeout-capable
process runner instead of calling Command::output() directly. Ensure the runner
terminates the child when the deadline is exceeded and propagates a suitable
error, while preserving each helper’s existing command and output parsing
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 Plus
Run ID: 03d2532e-b778-43f6-889b-106d206afc1a
📒 Files selected for processing (1)
app/src-tauri/src/process_recovery.rs
| let output = std::process::Command::new("powershell") | ||
| .args(["-NoProfile", "-NonInteractive", "-Command", script]) | ||
| .creation_flags(CREATE_NO_WINDOW) | ||
| .output() | ||
| .map_err(|e| format!("spawn powershell: {e}"))?; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching process_recovery/core_process/lib:\n'
git ls-files | rg 'app/src-tauri/src/(process_recovery|core_process)\.rs|app/src-tauri/src/lib\.rs' || true
printf '\nOutline process_recovery.rs relevant symbols:\n'
ast-grep outline app/src-tauri/src/process_recovery.rs --view compact || true
printf '\nLines 760-840 process_recovery.rs:\n'
sed -n '760,840p' app/src-tauri/src/process_recovery.rs | nl -ba -v760
printf '\nLines 150-195 lib.rs:\n'
sed -n '150,195,196p' app/src-tauri/src/lib.rs | nl -ba -v150 | sed -n '1,80p'
printf '\nLines 660-760 core_process.rs (if exists):\n'
sed -n '660,760p' app/src-tauri/src/core_process.rs | nl -ba -v660
printf '\nSearch for timeouts/deadlines around recovery/enumerate functions:\n'
rg -n "timeout|deadline|Duration|instant|enumerate_via_wmic|enumerate_via_cim|recover|recovery|Command::output|powershell|WMIC|CIM" app/src-tauri/src/process_recovery.rs app/src-tauri/src/core_process.rs app/src-tauri/src/lib.rs || trueRepository: tinyhumansai/openhuman
Length of output: 582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching process_recovery/core_process/lib:\n'
git ls-files | rg 'app/src-tauri/src/(process_recovery|core_process)\.rs|app/src-tauri/src/lib\.rs' || true
printf '\nOutline process_recovery.rs relevant symbols:\n'
ast-grep outline app/src-tauri/src/process_recovery.rs --view expanded | sed -n '1,220p'
printf '\nLines 760-840 process_recovery.rs:\n'
sed -n '760,840p' app/src-tauri/src/process_recovery.rs | awk '{printf "%6d\t%s\n", NR+759, $0}'
printf '\nLines 150-195 lib.rs:\n'
sed -n '150,195p' app/src-tauri/src/lib.rs | awk '{printf "%6d\t%s\n", NR+149, $0}'
printf '\nLines 660-760 core_process.rs:\n'
sed -n '660,760p' app/src-tauri/src/core_process.rs | awk '{printf "%6d\t%s\n", NR+659, $0}'
printf '\nSearch for timeouts/deadlines and related calls:\n'
rg -n "timeout|deadline|Duration|Instant|enumerate_via_wmic|enumerate_via_cim|recover|recovery|Command::output|powershell|WMIC|CIM|core_process" app/src-tauri/src/process_recovery.rs app/src-tauri/src/core_process.rs app/src-tauri/src/lib.rs || trueRepository: tinyhumansai/openhuman
Length of output: 32297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant command registration:\n'
sed -n '820,845p' app/src-tauri/src/lib.rs | awk '{printf "%6d\t%s\n", NR+819, $0}'
printf '\nCommand call sites for process_diagnostics_list_owned:\n'
rg -n "process_diagnostics_list_owned|invoke|list_.*diagnostics|diagnostics_list" . || true
printf '\nCommand call sites for recover_port_conflict:\n'
rg -n "recover_port_conflict" . || true
printf '\nBehavioral model: Command without timeout waiting on hanging child.\n'
python3 - <<'PY'
from pathlib import Path
p = Path('app/src-tauri/src/process_recovery.rs')
text = p.read_text()
checks = {
"enumerate_via_cim_spawns_powershell": 'let output = std::process::Command::new("powershell")' in text,
"enumerate_via_cim_uses_output": '.output()' in text and 'fn enumerate_via_cim()' in text[text.find('fn enumerate_via_cim'): text.find('fn enumerate_via_cim')+500],
"enumerate_all_calls": 'match enumerate_via_wmic()' in text and 'enumerate_via_cim()' in text,
"reap_calls_enumerate": 'enumerate_openhuman_processes()' in text,
"recover_block": 'tokio::task::spawn_blocking(crate::process_recovery::reap_stale_openhuman_processes)' in text,
"diagnostics_block": 'process_recovery::enumerate_openhuman_processes()' in text,
}
for k, v in checks.items():
print(f"{k}: {v}")
PYRepository: tinyhumansai/openhuman
Length of output: 50379
Add a timeout for Windows process enumeration.
enumerate_via_cim() uses Command::output() directly for the PowerShell child, so PowerShell or a hung CIM provider can block indefinitely. process_diagnostics_list_owned() calls this synchronously in app/src-tauri/src/lib.rs, and reap_stale_openhuman_processes() also calls it on the Windows recovery path. Use one timeout-capable runner for enumerate_via_wmic() and enumerate_via_cim() that terminates the child after the deadline.
🤖 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 `@app/src-tauri/src/process_recovery.rs` around lines 807 - 811, Update the
Windows enumeration helpers enumerate_via_wmic() and enumerate_via_cim() to use
a shared timeout-capable process runner instead of calling Command::output()
directly. Ensure the runner terminates the child when the deadline is exceeded
and propagates a suitable error, while preserving each helper’s existing command
and output parsing behavior.
PowerShell writes stdout in the console OEM code page, so a process path outside that page is replaced with `?` before Rust sees it and no decode can recover it. Pinning OutputEncoding makes the CIM path round-trip. Adds debug logging on both enumeration branches, per the logging rules in AGENTS.md.
|
Both fair. Fixed in f053879. The encoding one is worse than mangling — PowerShell replaces the character before it reaches Rust, so it's lost, not mis-decoded. Measured here:
Added Couldn't run |
The fallback only fires where wmic is absent, so the branch that matters is the one CI never reaches and no test exercised it. select_enumeration now takes both enumerators as arguments, so the choice can be driven directly without spawning anything, and the CIM script is a named constant rather than an inline literal so its contents can be asserted. Six tests: wmic results used with CIM untouched, CIM on wmic error, CIM on empty wmic output, CIM failure propagating rather than returning an empty list, the script keeping its UTF-8 pin and every parsed field, and the parser reading CIM blocks with a non-ASCII path and empty fields. Cannot run cargo here: the CEF build script needs cmake. Compiled and ran the extracted logic with the same tests standalone under rustc, all twelve checks pass, and rustfmt parses the file clean.
|
Tests added in 19a3810.
Still can't run |
The fixture was named for non-ASCII and asserted "non-ASCII path was mangled", but the path was C:\Users\Ordner\OpenHuman.exe, which is entirely ASCII. The assertion could not fail whatever the encoding did. Uses Müller (representable in cp1252) and 用户 (not), so a regression that drops the OutputEncoding pin fails on one or the other whichever code page the runner uses, and compares the whole string rather than a substring.
|
Good catch on the fixture. 573eb47 uses |
1 participant
enumerate_all_processesshells out towmic, which Microsoft removed in Windows 11 24H2. On those builds the spawn fails and process recovery sees no processes at all, so it can't find or reattach to anything.Tries
wmicfirst so older builds are unaffected, then falls back toGet-CimInstance Win32_Processvia PowerShell. The fallback emits the sameKey=Valueblocks separated by blank lines thatwmic /format:listproduced, soparse_wmic_list_outputhandles both without changes.Checked the fallback against real output on Windows 11: 280 records parsed, 279 with a pid (pid 0 is System Idle Process), 280 with a command, 278 with a parent — the two without are pid 0 and pid 4, which legitimately have none.
I couldn't build the whole crate to confirm —
cef-dll-sysneeds CMake and I don't have it installed — so this hasn't been compiled, only parsed and format-checked with rustfmt. Worth a CI run before merging.Summary by CodeRabbit