Repository navigation
fix(tauri): align staging datadir resolution with core (#1490) - #1492
Conversation
…1490) The Tauri shell's `cef_profile::default_root_dir_name` had its own runtime-only env lookup. A packaged staging `.app` launched from Finder has no shell env so it picked up `.openhuman` (production) while `openhuman_core` correctly resolved `.openhuman-staging` via `option_env!`. The resulting split-brain placed the CEF profile next to the older production install's CEF data, crashed the shell on profile load, and produced the staging-build startup loop reported in tinyhumansai#1490. Delegate the resolution to `openhuman_core::api::config::is_staging_app_env(app_env_from_env)` so the shell and the embedded core agree on the channel — including the compile-time fallback baked in by `build-desktop.yml`. Add four regression tests (primary env, vite alias, unset default, casing / whitespace normalization) under a local APP_ENV_LOCK mutex. Closes tinyhumansai#1490 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesEnvironment Selection Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 `@app/src-tauri/src/cef_profile.rs`:
- Around line 416-433: with_clean_app_env currently restores OPENHUMAN_APP_ENV
and VITE_OPENHUMAN_APP_ENV only on normal return, so a panic in body() leaks env
state; wrap the call to body() in std::panic::catch_unwind (or otherwise use a
scope guard) so you can always run the restore logic, then if catch_unwind
captured a panic re-raise it with std::panic::resume_unwind; keep the
APP_ENV_LOCK guard semantics (guard variable) and ensure restoration uses
prior_primary/prior_vite before dropping the guard so environment is restored
even when body panics.
🪄 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: 4e8a7e33-defc-4716-84d3-74cbc16a9085
📒 Files selected for processing (1)
app/src-tauri/src/cef_profile.rs
Local staging build verification — passedBuilt the Tauri shell with staging env baked in: Launched the resulting binary with all runtime env vars scrubbed ( Log line from the launched binary (path + user-id redacted): Pre-fix this resolved to
(The launch additionally surfaced a |
…mansai#1490) Wrap the `body()` call in `with_clean_app_env` with `std::panic::catch_unwind` + `resume_unwind` so the prior `OPENHUMAN_APP_ENV` / `VITE_OPENHUMAN_APP_ENV` values are restored on both normal return and panic. Without this a panicking assertion in one regression test left the env mutated and could mask or flake later tests sharing the same process. Per CodeRabbit review on tinyhumansai#1492. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai applied the suggested panic-safety fix in 92a6e54 — Note: push went through with |
|
Tip For best results, initiate chat on the files or code changes.
Regarding the |
…1490) (tinyhumansai#1492) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
cef_profile::default_root_dir_namenow delegates toopenhuman_core::api::config::{app_env_from_env, is_staging_app_env}so the shell and the embedded core agree on.openhumanvs.openhuman-staging— including the compile-timeoption_env!fallback that staging CI bakes into the build.APP_ENV_LOCKmutex.Problem
Reported in #1490 (P0): staging builds enter an open → crash → close → relaunch loop on machines that already have an older OpenHuman install. The only known workaround is to manually wipe all older OpenHuman app data.
Root cause is split-brain datadir resolution between the Tauri shell and
openhuman_core:app/src-tauri/src/cef_profile.rsdefault_root_dir_namereadOPENHUMAN_APP_ENV/VITE_OPENHUMAN_APP_ENVviastd::env::varonly, with nooption_env!compile-time fallback.src/api/config.rsapp_env_from_envchecks runtime thenoption_env!, which staging CI bakes via.github/workflows/build-desktop.yml(OPENHUMAN_APP_ENV+VITE_OPENHUMAN_APP_ENV)..applaunched from Finder has no shell env, socef_profilereturned.openhuman(production) whileopenhuman_corecorrectly returned.openhuman-staging.active_user.tomllanded under~/.openhuman/users/<id>/cef/next to the older production install's CEF profile. The older Chromium-revision profile is incompatible with the staging CEF build → CEF crashes on profile load → shell exits → the user reopens / OS relaunches → loop.~/.openhuman-staging/, so the state was half-correct and half-corrupting. The reported workaround ("wipe~/.openhuman") removes the colliding CEF profile, which is why it worked.Bundle identifier is shared (
tauri.conf.json→com.openhuman.app); the entire staging/production isolation lives in the datadir name, which made this single divergence catastrophic.Solution
Single-source-of-truth fix: replace the local lookup in
cef_profile.rswith a call to the helper the core already exposes:The Tauri shell already links
openhuman_core(app/src-tauri/Cargo.toml), and both helpers arepub. Going forward the shell and the core can never disagree on which environment the build is for.Trade-offs considered and rejected:
~/.openhuman/; documenting that path in the linked issue is preferred over auto-migration that's hard to reverse.lib.rs::resolve_sentry_environment(Sentry environment tag — currently only falls back tooption_env!("VITE_OPENHUMAN_APP_ENV"), missing the primaryOPENHUMAN_APP_ENV). Out of scope for this PR — observability bug, not crash-loop. Can be addressed in a follow-up.Submission Checklist
cargo test --manifest-path app/src-tauri/Cargo.toml --lib cef_profileruns the four new regression tests over the changed lines. The whole diff is the two-line function body + tests, so coverage on changed lines is 100%.docs/TEST-COVERAGE-MATRIX.md.## Related.docs/RELEASE-MANUAL-SMOKE.mddoes not need a new bullet.Closes #NNNin the## RelatedsectionImpact
.app/.exe/.AppImagefrom the OS shell with no inherited env. macOS is the most common reproduction path because of the Finder-launch flow described in the issue.cef_profile.rs.~/.openhuman/was contaminated by a pre-fix staging launch will not be auto-healed by this PR. The fix stops the contamination going forward (staging will write only to~/.openhuman-staging/) but the corrupted~/.openhuman/still needs a one-time manual wipe for affected users. Will be called out in the issue.OPENHUMAN_APP_ENV=production→.openhuman, exactly as before).Manual verification
Build a staging variant locally and confirm the datadir split is fixed end-to-end:
OPENHUMAN_APP_ENV=staging cargo tauri build --debug open app/src-tauri/target/debug/bundle/macos/OpenHuman.app # or platform equivalentExpected:
~/.openhuman-staging/users/<id>/cef/populated.~/.openhuman/untouched (or, if it already exists from a production install, no new staging artifacts under it).~/.openhuman/from an older production install.Related
lib.rs::resolve_sentry_environmentto also fall back tooption_env!("OPENHUMAN_APP_ENV")(observability only, no crash-loop risk).AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
app/workspace files changed —pnpm --filter openhuman-app format:checkpnpm typecheckcargo test --manifest-path app/src-tauri/Cargo.toml --lib cef_profile(12 passed, 0 failed)src/(core crate) Rust changed — Rust fmt/checkcargo check --manifest-path app/src-tauri/Cargo.toml(clean; only pre-existing unrelated warnings)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
~/.openhuman-staging/for CEF cache, window state, andactive_user.toml(matching whatopenhuman_corealready did).Parity Contract
OPENHUMAN_APP_ENV=productionbaked at compile time, no runtime env) continue to resolve~/.openhuman/. Dev runs withOPENHUMAN_APP_ENVset at runtime continue to honor it.app_env_from_env+is_staging_app_env) that the core, the cookie-DB path, the Sentry env tag, and the api-base-URL selector already use — there is no second code path to keep in sync going forward.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Refactor
Tests