From 69c7eaa91353aaa474aba178e800abb72279980e Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 16 Jul 2026 17:38:56 -0700 Subject: [PATCH 1/2] feat(app): implement EnvironmentPolicy::LoginShell PATH repair GUI-launched apps on macOS and Linux inherit a minimal PATH that omits login-shell entries (Homebrew, version-manager shims), so tools the app shells out to cannot be found. Wire EnvironmentPolicy::LoginShell to the fix-path-env crate (Tauri's login-shell repair, Apache-2.0 OR MIT) and apply it in Preflight before anything is spawned. The repair mutates the process environment via std::env::set_var, which is UB on Unix under concurrent env access. The variant's rustdoc makes the precondition the caller's explicit obligation: select it only from a single-threaded main() before any thread spawns. This is the same objection that keeps a Custom(vars) variant out of the enum; it is accepted here only because the repair is a single vetted operation, not an open-ended env hook. A failed repair is logged and falls back to the inherited environment, never aborting startup. fix-path-env is pulled in only on macOS/Linux; Windows GUI processes inherit PATH correctly, so the policy is a documented no-op there. --- Cargo.lock | 29 ++++++++++ Cargo.toml | 3 ++ crates/app/Cargo.toml | 6 +++ crates/app/src/shell.rs | 114 ++++++++++++++++++++++++++++++++++++---- 4 files changed, 141 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ae3a0d7a..0453071e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2299,6 +2299,16 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" +[[package]] +name = "fix-path-env" +version = "0.0.0" +source = "git+https://github.com/tauri-apps/fix-path-env-rs?rev=c4c45d503ea115a839aae718d02f79e7c7f0f673#c4c45d503ea115a839aae718d02f79e7c7f0f673" +dependencies = [ + "home", + "strip-ansi-escapes", + "thiserror 1.0.69", +] + [[package]] name = "fixedbitset" version = "0.5.7" @@ -3182,6 +3192,7 @@ name = "gpui-component-app" version = "0.5.1" dependencies = [ "anyhow", + "fix-path-env", "gpui", "gpui-component", "gpui-component-assets", @@ -7784,6 +7795,15 @@ dependencies = [ "quote", ] +[[package]] +name = "strip-ansi-escapes" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a8f8038e7e7969abb3f1b7c2a811225e9296da208539e0f79c5251d6cac0025" +dependencies = [ + "vte", +] + [[package]] name = "strum" version = "0.27.2" @@ -9336,6 +9356,15 @@ dependencies = [ "libc", ] +[[package]] +name = "vte" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "231fdcd7ef3037e8330d8e17e61011a2c244126acc0a982f4040ac3f9f0bc077" +dependencies = [ + "memchr", +] + [[package]] name = "waker-fn" version = "1.2.0" diff --git a/Cargo.toml b/Cargo.toml index fcf7e401..66c1f602 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,6 +61,9 @@ ctor = "0.4.0" derive_more = "0.99.17" dirs = "6" env_logger = "0.11" +# Login-shell PATH repair for GUI-launched apps (EnvironmentPolicy::LoginShell). +# Git-only crate (no crates.io release); pinned to the same rev agent-term uses. +fix-path-env = { git = "https://github.com/tauri-apps/fix-path-env-rs", rev = "c4c45d503ea115a839aae718d02f79e7c7f0f673" } futures = "0.3" image = "0.25.1" inventory = "0.3.19" diff --git a/crates/app/Cargo.toml b/crates/app/Cargo.toml index ac8949be..3d41d98a 100644 --- a/crates/app/Cargo.toml +++ b/crates/app/Cargo.toml @@ -18,6 +18,12 @@ serde.workspace = true thiserror.workspace = true toml.workspace = true +# EnvironmentPolicy::LoginShell's PATH repair. macOS/Linux only: Windows GUI +# processes inherit PATH from the launching context, so the policy is a +# documented no-op there and the crate is not pulled in. +[target.'cfg(any(target_os = "macos", target_os = "linux"))'.dependencies] +fix-path-env.workspace = true + [dev-dependencies] serde_json.workspace = true tempfile.workspace = true diff --git a/crates/app/src/shell.rs b/crates/app/src/shell.rs index 917c6838..63dce0c7 100644 --- a/crates/app/src/shell.rs +++ b/crates/app/src/shell.rs @@ -31,17 +31,42 @@ pub enum EnvironmentPolicy { Inherit, /// Repair `PATH` from the user's login shell. /// - /// Not implemented in core: `fix-path-env` is not a dependency. Selecting - /// this logs a warning and behaves as [`EnvironmentPolicy::Inherit`]; apps - /// that need it should run their own repair in `before_platform`. + /// GUI-launched processes on macOS (Finder/Dock/`launchd`) and Linux + /// (desktop launchers) inherit a minimal `PATH` that omits entries a login + /// shell would add (`/opt/homebrew/bin`, version-manager shims, …), so tools + /// the app shells out to cannot be found. This policy runs the login shell + /// once during `Preflight` and copies its environment into the current + /// process — the established desktop-app fix (used by Tauri and others). + /// + /// # Soundness precondition (the caller's obligation) + /// + /// The repair mutates the process environment through `std::env::set_var`, + /// which is **Undefined Behavior on Unix if any other thread may read or + /// write the environment concurrently**. The shell cannot prove the caller + /// has not already spawned threads, so the obligation is yours: **select + /// `LoginShell` only from a single-threaded `main()`, before any thread is + /// spawned** (the typical first statements of `main`). + /// + /// This is the same objection that keeps a general `Custom(vars)` variant out + /// of this enum (see the note below it). It is accepted here only because the + /// repair is a single, vetted, widely-used operation rather than an + /// open-ended env hook — the alternative is every app hand-rolling the same + /// unsafe mutation. + /// + /// Failure to read the login shell is **non-fatal**: it is logged and the + /// process keeps the inherited environment (behaves as + /// [`EnvironmentPolicy::Inherit`]); it never aborts startup. On Windows there + /// is no login-shell `PATH` to repair, so this is a documented no-op. LoginShell, } // There is deliberately no `Custom(vars)` variant: `std::env::set_var` is // unsafe (UB with concurrent environment access on Unix), and this builder -// cannot know whether the caller already spawned threads. Apps that need to -// mutate the environment must do so in their own `main()` before constructing -// the shell, where the safety obligation is visibly theirs. +// cannot know whether the caller already spawned threads. The one env mutation +// the shell performs — `LoginShell` above — is a single vetted repair carrying an +// explicit caller precondition, not an open-ended "set these vars" hook. Apps +// that need arbitrary environment changes must do so in their own `main()` before +// constructing the shell, where the safety obligation is visibly theirs. /// Process-global logging policy (plan §3). The library must not seize the /// process logger by default. @@ -499,14 +524,35 @@ fn validate_identity(identity: &IdentityRef) -> Result<(), AppShellError> { fn apply_environment(policy: EnvironmentPolicy) { match policy { EnvironmentPolicy::Inherit => {} - EnvironmentPolicy::LoginShell => { - log::warn!( - "EnvironmentPolicy::LoginShell is not implemented in core; inheriting environment" - ); - } + EnvironmentPolicy::LoginShell => apply_login_shell(), } } +/// Repair `PATH` from the login shell. Soundness rests on the caller precondition +/// documented on [`EnvironmentPolicy::LoginShell`] (no other threads yet). +#[cfg(any(target_os = "macos", target_os = "linux"))] +fn apply_login_shell() { + // `fix_path_env::fix()` copies the login shell's environment into this + // process via `std::env::set_var`; the caller precondition (single-threaded + // `main`) is what makes that sound. A failed repair is non-fatal — keep the + // inherited environment rather than aborting startup. + if let Err(err) = fix_path_env::fix() { + log::warn!( + "EnvironmentPolicy::LoginShell: could not repair PATH from the login \ + shell; keeping the inherited environment: {err}" + ); + } +} + +/// No login-shell `PATH` to repair off Unix; documented no-op (see the variant). +#[cfg(not(any(target_os = "macos", target_os = "linux")))] +fn apply_login_shell() { + log::debug!( + "EnvironmentPolicy::LoginShell is a no-op on this platform; keeping the \ + inherited environment" + ); +} + fn apply_logging(policy: LoggingPolicy, paths: &AppPaths) { match policy { LoggingPolicy::External => {} @@ -545,3 +591,49 @@ impl AssetSource for ChainedAssets { Ok(out) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn identity() -> IdentityRef { + IdentityRef { + app_id: "com.example.envtest", + display_name: "Env Test", + data_namespace: "envtest", + binary_name: None, + org: None, + publisher: None, + url_schemes: &[], + categories: &[], + macos: None, + linux: None, + windows: None, + legacy_ids: &[], + min_os: None, + version: "0.0.0", + cfbundle_short_version: "0.0.0", + msix_version: "0.0.0.0", + } + } + + #[test] + fn default_environment_policy_is_inherit() { + let builder = AppShellBuilder::new(identity()); + assert!(matches!(builder.environment, EnvironmentPolicy::Inherit)); + } + + #[test] + fn environment_setter_records_login_shell() { + let builder = AppShellBuilder::new(identity()).environment(EnvironmentPolicy::LoginShell); + assert!(matches!(builder.environment, EnvironmentPolicy::LoginShell)); + } + + #[test] + fn inherit_environment_is_a_noop() { + // Inherit must never shell out or mutate the environment. Applying it is a + // pure no-op; the LoginShell path is intentionally not exercised here (it + // would spawn the login shell and mutate process env in CI). + apply_environment(EnvironmentPolicy::Inherit); + } +} From 06cd016b2a65ac8ec499b48a54cf73002b7b8619 Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 16 Jul 2026 17:39:04 -0700 Subject: [PATCH 2/2] fix(app-manifest): make include_identity! dependency-free via $crate The build.rs-generated identity source hardcoded the absolute path ::gpui_component_manifest::schema::IdentityRef, forcing every consuming app to carry gpui-component-manifest as a runtime dependency (plus a cargo-machete ignore) even when it only reached the macro through gpui-component-app's re-export. Emit the schema types unqualified and have include_identity! expand to a private module that imports them with `use $crate::schema::*` before include!()-ing the generated file. $crate resolves to the defining crate by def-id, so it works transitively through a re-export: an app calling gpui_component_app::include_identity!() now needs the manifest crate only as a build-dependency. Drop the direct runtime dep and machete ignore from both example apps, and switch the downstream fixture to reach the macro through a local re-export shim so it proves the dependency-free path in a hermetic test. --- crates/app-manifest/src/build.rs | 45 ++++++++++++++++--- crates/app-manifest/src/lib.rs | 34 ++++++++++++-- .../tests/fixtures/downstream-app/Cargo.toml | 8 +++- .../manifest-reexport/Cargo.toml | 11 +++++ .../manifest-reexport/src/lib.rs | 6 +++ .../tests/fixtures/downstream-app/src/main.rs | 5 ++- examples/app_shell/Cargo.toml | 10 ++--- examples/app_shell_tray/Cargo.toml | 10 ++--- 8 files changed, 104 insertions(+), 25 deletions(-) create mode 100644 crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/Cargo.toml create mode 100644 crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/src/lib.rs diff --git a/crates/app-manifest/src/build.rs b/crates/app-manifest/src/build.rs index c6f02132..1b9fb43c 100644 --- a/crates/app-manifest/src/build.rs +++ b/crates/app-manifest/src/build.rs @@ -63,10 +63,16 @@ fn generate_source( cfbundle_short_version: &str, msix_version: &str, ) -> String { + // Types are named UNQUALIFIED here. `include_identity!` includes this file + // inside a module that brings the schema types into scope via `$crate`, so + // the identifiers resolve to `gpui-component-manifest` even when the macro is + // reached through a re-export — letting apps depend on it only as a + // build-dependency. Do not reintroduce `::gpui_component_manifest::` paths: + // an absolute path would force every consumer back onto a direct runtime dep. let mut source = String::new(); writeln!( source, - "pub static APP_IDENTITY: ::gpui_component_manifest::schema::IdentityRef = ::gpui_component_manifest::schema::IdentityRef {{" + "pub static APP_IDENTITY: IdentityRef = IdentityRef {{" ) .expect("writing generated source to a String cannot fail"); write_field(&mut source, "app_id", &format!("{:?}", identity.app_id)); @@ -140,7 +146,7 @@ fn macos_source(value: Option<&MacosIdentity>) -> String { |value| { let usage_strings: Vec<_> = value.usage_strings.iter().collect(); format!( - "Some(::gpui_component_manifest::schema::MacosIdentityRef {{ category: {:?}, entitlements: &{:?}, usage_strings: &{:?} }})", + "Some(MacosIdentityRef {{ category: {:?}, entitlements: &{:?}, usage_strings: &{:?} }})", value.category, value.entitlements, usage_strings ) }, @@ -152,7 +158,7 @@ fn linux_source(value: Option<&LinuxIdentity>) -> String { || "None".to_owned(), |value| { format!( - "Some(::gpui_component_manifest::schema::LinuxIdentityRef {{ categories: &{:?}, desktop_keywords: &{:?} }})", + "Some(LinuxIdentityRef {{ categories: &{:?}, desktop_keywords: &{:?} }})", value.categories, value.desktop_keywords ) }, @@ -164,7 +170,7 @@ fn windows_source(value: Option<&WindowsIdentity>) -> String { || "None".to_owned(), |value| { format!( - "Some(::gpui_component_manifest::schema::WindowsIdentityRef {{ publisher: {:?} }})", + "Some(WindowsIdentityRef {{ publisher: {:?} }})", value.publisher ) }, @@ -176,7 +182,7 @@ fn min_os_source(value: Option<&MinOsVersions>) -> String { || "None".to_owned(), |value| { format!( - "Some(::gpui_component_manifest::schema::MinOsVersionsRef {{ macos: {:?}, windows: {:?}, linux: {:?} }})", + "Some(MinOsVersionsRef {{ macos: {:?}, windows: {:?}, linux: {:?} }})", value.macos, value.windows, value.linux ) }, @@ -208,4 +214,33 @@ mod tests { assert!(source.contains(r#"display_name: "Quoted \"Name\"\n""#)); assert!(source.contains(r#"binary_name: Some("bin\\name")"#)); } + + #[test] + fn generated_source_names_schema_types_unqualified() { + // `include_identity!` resolves these via `$crate`, so the generated file + // must NOT hardcode `::gpui_component_manifest::` — that would force a + // direct runtime dependency on every consumer (see the macro's rustdoc). + let identity = AppIdentity { + app_id: "com.example.macro".to_owned(), + display_name: "Macro".to_owned(), + data_namespace: "macro".to_owned(), + binary_name: None, + org: None, + publisher: None, + url_schemes: Vec::new(), + categories: Vec::new(), + macos: Some(MacosIdentity::default()), + linux: None, + windows: None, + legacy_ids: Vec::new(), + min_os: None, + }; + let source = generate_source(&identity, "1.0.0", "1.0.0", "1.0.0.0"); + assert!(source.contains("APP_IDENTITY: IdentityRef = IdentityRef {")); + assert!(source.contains("Some(MacosIdentityRef {")); + assert!( + !source.contains("::gpui_component_manifest"), + "generated source must not hardcode an absolute manifest path:\n{source}" + ); + } } diff --git a/crates/app-manifest/src/lib.rs b/crates/app-manifest/src/lib.rs index c248cedd..3497277a 100644 --- a/crates/app-manifest/src/lib.rs +++ b/crates/app-manifest/src/lib.rs @@ -8,15 +8,41 @@ pub mod versions; pub use error::ManifestError; -/// Includes identity source generated for the consuming application. +/// Includes identity source generated for the consuming application, defining a +/// `pub static APP_IDENTITY` at the call site. /// /// Call this from the consuming application's own crate after its `build.rs` has /// called [`build::emit_identity`]. Do not call it from a library crate: `OUT_DIR` -/// must belong to the application. The planned `gpui-component-app` crate will -/// re-export this macro for ergonomic access. +/// must belong to the application. `gpui-component-app` re-exports this macro for +/// ergonomic access. +/// +/// # No direct manifest dependency required +/// +/// The generated source names its schema types unqualified (`IdentityRef`, …). +/// This macro includes that source inside a private module that imports those +/// types with `use $crate::schema::*`. `$crate` resolves to the crate that +/// *defined* the macro — `gpui-component-manifest` — by def-id rather than the +/// caller's extern prelude, so it works even when the macro is reached through a +/// re-export (e.g. `gpui_component_app::include_identity!`). Consequently an app +/// that calls this via `gpui-component-app` needs `gpui-component-manifest` only +/// as a `[build-dependencies]` entry (for [`build::emit_identity`]) — never as a +/// runtime dependency. #[macro_export] macro_rules! include_identity { () => { - include!(concat!(env!("OUT_DIR"), "/gpui_app_identity.rs")); + // The private module scopes the `use` so the generated file's unqualified + // type names resolve via `$crate`; `pub use` then lifts `APP_IDENTITY` + // back to the call site. `unused_imports` is allowed because an identity + // without a given platform section never references that section's type. + #[doc(hidden)] + mod __gpui_app_identity { + #[allow(unused_imports)] + use $crate::schema::{ + IdentityRef, LinuxIdentityRef, MacosIdentityRef, MinOsVersionsRef, + WindowsIdentityRef, + }; + include!(concat!(env!("OUT_DIR"), "/gpui_app_identity.rs")); + } + pub use __gpui_app_identity::APP_IDENTITY; }; } diff --git a/crates/app-manifest/tests/fixtures/downstream-app/Cargo.toml b/crates/app-manifest/tests/fixtures/downstream-app/Cargo.toml index f21b493a..cd8136bc 100644 --- a/crates/app-manifest/tests/fixtures/downstream-app/Cargo.toml +++ b/crates/app-manifest/tests/fixtures/downstream-app/Cargo.toml @@ -4,8 +4,13 @@ version = "0.4.2" edition = "2024" publish = false +# No runtime dependency on gpui-component-manifest. This fixture proves that an +# app reaching `include_identity!` through a re-export (here the local +# `manifest-reexport` shim, mirroring gpui-component-app) needs the manifest +# crate only as a BUILD dependency: macro `$crate` resolves the schema types to +# gpui-component-manifest transitively via the shim. [dependencies] -gpui-component-manifest = { path = "../../.." } +manifest-reexport = { path = "manifest-reexport" } [build-dependencies] gpui-component-manifest = { path = "../../.." } @@ -17,3 +22,4 @@ url_schemes = ["fixtureapp"] categories = ["Development"] [workspace] +members = ["manifest-reexport"] diff --git a/crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/Cargo.toml b/crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/Cargo.toml new file mode 100644 index 00000000..14e0166a --- /dev/null +++ b/crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "manifest-reexport" +version = "0.0.0" +edition = "2024" +publish = false + +# Mirrors how gpui-component-app re-exports the identity macro. The downstream +# fixture binary reaches `include_identity!` through this shim, proving that a +# consumer needs the manifest crate only as a build dependency. +[dependencies] +gpui-component-manifest = { path = "../../../.." } diff --git a/crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/src/lib.rs b/crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/src/lib.rs new file mode 100644 index 00000000..a55eed14 --- /dev/null +++ b/crates/app-manifest/tests/fixtures/downstream-app/manifest-reexport/src/lib.rs @@ -0,0 +1,6 @@ +//! Minimal re-export shim mirroring `gpui-component-app`'s re-export of the +//! identity macro. Its only job is to prove that reaching `include_identity!` +//! through a re-export needs no direct `gpui-component-manifest` dependency in +//! the calling crate: macro `$crate` still resolves to the defining manifest +//! crate, transitively available via this shim. +pub use gpui_component_manifest::include_identity; diff --git a/crates/app-manifest/tests/fixtures/downstream-app/src/main.rs b/crates/app-manifest/tests/fixtures/downstream-app/src/main.rs index 68a49e82..bb9284b3 100644 --- a/crates/app-manifest/tests/fixtures/downstream-app/src/main.rs +++ b/crates/app-manifest/tests/fixtures/downstream-app/src/main.rs @@ -1,4 +1,7 @@ -gpui_component_manifest::include_identity!(); +// Reaches `include_identity!` through the `manifest-reexport` shim rather than +// naming `gpui_component_manifest` directly — proving the re-export path compiles +// with the manifest crate present only as a build dependency (see Cargo.toml). +manifest_reexport::include_identity!(); fn main() { assert_eq!(APP_IDENTITY.app_id, "com.example.downstreamfixture"); diff --git a/examples/app_shell/Cargo.toml b/examples/app_shell/Cargo.toml index 923d876b..1cc00273 100644 --- a/examples/app_shell/Cargo.toml +++ b/examples/app_shell/Cargo.toml @@ -12,17 +12,13 @@ categories = ["Development"] [dependencies] gpui-component-app.workspace = true -# Referenced by the `include_identity!()`-generated `APP_IDENTITY` (its type is -# `::gpui_component_manifest::schema::IdentityRef`), so it is a runtime dep even -# though no source line names it — hence the cargo-machete ignore below. -gpui-component-manifest.workspace = true serde.workspace = true +# Only a build dependency: `build.rs` calls `emit_identity()`. The runtime +# `include_identity!()` (re-exported by gpui-component-app) resolves the manifest +# schema types via macro `$crate`, so no runtime dependency is needed here. [build-dependencies] gpui-component-manifest.workspace = true -[package.metadata.cargo-machete] -ignored = ["gpui-component-manifest"] - [lints] workspace = true diff --git a/examples/app_shell_tray/Cargo.toml b/examples/app_shell_tray/Cargo.toml index b6c10be2..9841018c 100644 --- a/examples/app_shell_tray/Cargo.toml +++ b/examples/app_shell_tray/Cargo.toml @@ -12,16 +12,12 @@ categories = ["Development"] [dependencies] gpui-component-app.workspace = true -# Referenced by the `include_identity!()`-generated `APP_IDENTITY` (its type is -# `::gpui_component_manifest::schema::IdentityRef`), so it is a runtime dep even -# though no source line names it — hence the cargo-machete ignore below. -gpui-component-manifest.workspace = true +# Only a build dependency: `build.rs` calls `emit_identity()`. The runtime +# `include_identity!()` (re-exported by gpui-component-app) resolves the manifest +# schema types via macro `$crate`, so no runtime dependency is needed here. [build-dependencies] gpui-component-manifest.workspace = true -[package.metadata.cargo-machete] -ignored = ["gpui-component-manifest"] - [lints] workspace = true