fix: spawn desktop engine with resolved env, not the inherited one

[praisonai-triage-agent]

Fixes #4500

Problem

venv_resolve::spawn_env — the whole PYTHONPATH/PYTHONHOME/PYTHONSTARTUP defence — had no production caller. Every caller was a test. supervisor::start inherited the shell's environment and only added PYTHONUTF8/PYTHONIOENCODING, so a user with PYTHONHOME or PYTHONPATH exported (conda, distro tooling, a systemd user session) got the engine running against the wrong stdlib or a foreign site-packages, and the module written to prevent exactly that never ran.

Fix (minimal, no new API surface)

• supervisor::start now takes the resolved env: &BTreeMap<String, String> and applies it with env_clear() + envs(env) before its existing UTF-8 vars.
• main.rs builds that env from the VenvLayout it already resolves, via spawn_env, collecting std::env::vars() after setting PRAISONAI_APP_BUNDLE so that var survives the clear.
• Regression test in tests/real_venvs.rs: with PYTHONHOME=/nonexistent and a poisoned PYTHONPATH in the inherited map, asserts neither survives into the spawn env and — driven through the real resolved interpreter — that the spawned child sees neither. Skips silently when no venv is present, matching the existing test.

Scope

Change is confined to src/praisonai-desktop/src-tauri (the Tauri desktop shell), a separate crate from the out-of-scope src/praisonai-rust. No changes to the Python or TypeScript SDKs.

Test plan

• cargo test -p praisonai-desktop on a machine with a venv present (CI runners skip the venv-dependent test)

Generated with Claude Code

[praisonai-triage-agent]

@coderabbitai review

[praisonai-triage-agent]

/review

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[greptile-apps]

Greptile Summary

The PR routes desktop-engine startup through the resolved virtual-environment map and clears inherited Python-specific poisoning variables. Its Unicode-safe collection strategy also drops required variables containing undecodable bytes, which can leave engine subprocess utilities unavailable.

• Builds the child environment with spawn_env after configuring the application bundle.
• Makes the supervisor clear and replace the inherited process environment.
• Adds a real-virtual-environment regression test for poisoned PYTHONHOME and PYTHONPATH.

Confidence Score: 4/5

The PR should not merge until non-Unicode environment handling preserves operational variables needed by engine subprocesses.

The panic is removed, but a non-Unicode PATH is now silently discarded before env_clear(), leaving the engine unable to discover system utilities used for process fingerprinting and credential storage.

Files Needing Attention: src/praisonai-desktop/src-tauri/src/main.rs

Important Files Changed

Filename	Overview
src/praisonai-desktop/src-tauri/src/main.rs	Constructs the resolved spawn environment without panicking on non-Unicode entries, but can discard operational variables such as PATH.
src/praisonai-desktop/src-tauri/src/supervisor.rs	Applies the supplied resolved environment after clearing inheritance, correctly enforcing the caller-provided map but amplifying any omitted entries.
src/praisonai-desktop/src-tauri/tests/real_venvs.rs	Verifies that poisoned Python environment variables are removed from both the resolved map and an actual child process.

_{Reviews (2): Last reviewed commit: "fix: collect inherited env with vars_os ..." | Re-trigger Greptile}

[MervinPraison]

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

Phase 1: Review per AGENTS.md

1. Protocol-driven: check heavy implementations vs core SDK
2. Backward compatible: ensure zero feature regressions
3. Performance: no hot-path regressions
4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. If all issues are resolved, approve the PR / close the Issue
12. If blocking issues remain, request changes / leave clear action items

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job

---

Final Architecture Review — PR #4519

• Read AGENTS.md & confirm scope
• Read changed files (main.rs, supervisor.rs, real_venvs.rs)
• Assess Greptile P1: non-Unicode env panics startup — VALID
• Fix valid issue & push (a482e76)
• Final verdict

---

Scope

This PR is confined to src/praisonai-desktop/src-tauri (the Tauri desktop shell crate). It touches neither the Python SDK (praisonaiagents/praisonai), the TypeScript SDK (src/praisonai-ts), nor the out-of-scope src/praisonai-rust. The AGENTS.md Agent-class / routing / core-vs-wrapper rules don't apply here — no core SDK surface changes, no new params, no new deps.

Phase 1 — Review

Check	Result
Adds real value?	✅ Yes. spawn_env (the PYTHONHOME/PYTHONPATH defence) had no production caller — every caller was a test. This wires it into supervisor::start, so the venv the resolver proves is actually the one the engine spawns against. Genuine robustness/security fix, not scope creep.
Backward compatible?	✅ supervisor::start gains one env: &BTreeMap param; the only production caller is updated. No public SDK surface.
Performance	✅ One-time env collection on engine start, not a hot path.
Minimal / no bloat	✅ No new API surface, no new knobs. env_clear() + envs(env) + existing UTF-8 vars.

Phase 2 — Issue found & fixed

Greptile P1 (valid) — main.rs:318 could abort startup. std::env::vars() panics on any non-Unicode env key/value on Unix/macOS, and the release profile sets panic = "abort" (Cargo.toml). A single non-Unicode inherited variable (a locale-encoded value, a foreign tool's export) would therefore abort the whole app before the engine started.

Fix (a482e76): collect via std::env::vars_os() and drop undecodable entries:

let inherited: std::collections::BTreeMap<String, String> = std::env::vars_os()
    .filter_map(|(key, value)| Some((key.into_string().ok()?, value.into_string().ok()?)))
    .collect();

spawn_env only accepts BTreeMap<String, String> anyway, so a non-UTF-8 variable is one Python could never have received — dropping it loses nothing and can no longer crash startup.

I independently reviewed supervisor.rs and real_venvs.rs: the env_clear() + envs(env) ordering (resolved env first, then fixed UTF-8/version overrides) is correct, and the regression test drives a real interpreter to confirm PYTHONHOME/PYTHONPATH are absent. No further issues.

Skipped: nothing actionable. Qodo posted only a paused-reviews notice (no findings). No CodeRabbit/Gemini/Copilot findings were present in the thread.

Files modified

• src/praisonai-desktop/src-tauri/src/main.rs — vars() → vars_os() with lossy filtering.

Note on verification

cargo is not in my allowed tools here, so I could not run cargo build/cargo test -p praisonai-desktop in this environment. The change is type-correct (into_string() → Result<String, OsString>, .ok()? → Option<String>, closure yields Option<(String, String)> for filter_map). Please let CI compile it.

Verdict

✅ Approve (pending CI green). The change is a legitimate, minimal robustness/security fix, and the sole blocking reviewer concern (non-Unicode env abort) is now resolved. I cannot submit a formal GitHub approval for security reasons, but from an architecture standpoint this is good to merge once cargo test -p praisonai-desktop passes.
• Branch

[greptile-apps]

Non-Unicode PATH is dropped

If the inherited PATH contains non-Unicode bytes, this conversion omits the entire variable; spawn_env then supplies only the virtual-environment bin directory before env_clear() replaces the child environment, causing engine operations that invoke system tools such as ps, security, or secret-tool by name to fail.