hero_service::HeroService::rpc/ui() should default-forward PATH_ROOT et al #51

New issue

Open

opened 2026-05-18 03:12:06 +00:00 by mik-tf · 0 comments

mik-tf commented

2026-05-18 03:12:06 +00:00

Owner

Surfaced by s110 hero_logic D-10 closure (hero_proc#102 D-10 sweep).

Problem

hero_service::HeroService::rpc() and ::ui() only seed RUST_LOG=info in env_vars. When the CLI invokes HeroServices::start(), the resulting ActionSpec carries only RUST_LOG. hero_proc spawns daemons with clean env, so under hero_lib 30a0b34e's strict PATH_ROOT regime, every spawned daemon panics at herolib_core::base::paths::path_root() (crates/core/src/base/paths.rs:38).

This is s109 lesson #19, restated for the hero_service abstraction layer.

Current workaround

Each CLI that uses HeroServices has to thread env vars manually:

fn forward_env(mut service: HeroService) -> HeroService {
    for key in ["PATH_ROOT", "PATH_VAR", "PATH_BUILD", "PATH_CODE", "HERO_SOCKET_DIR"] {
        if let Ok(v) = std::env::var(key)
            && !v.is_empty()
        {
            service = service.env(key, v);
        }
    }
    service
}

let services = HeroServices::new(SERVICE_NAME)
    .add(forward_env(HeroService::rpc("hero_logic_server")))
    .add(forward_env(HeroService::ui("hero_logic_admin").socket_name("admin.sock")));

Live example: hero_logic ba74b2b crates/hero_logic/src/main.rs.

Proposed fix

Move the forwarding into HeroService::rpc() and HeroService::ui() directly — default-seed PATH_ROOT/PATH_VAR/PATH_BUILD/PATH_CODE/HERO_SOCKET_DIR from the parent process env when set and non-empty. The few CLI consumers each strip ~6 lines, and every future consumer benefits without thinking about it.

Alternative: a single method .forward_hero_env() on HeroService that does the loop, opted into by callers.

Affected consumers (as of 2026-05-17)

hero_logic (s110, this commit)
s109 hero_indexer uses a different shape (raw hero_proc_sdk::ActionSpec with its own forward_env_if_set helper) — would also benefit if it migrated to HeroServices
Future D-10 sweeps with the CLI lifecycle pattern

Out of scope

The deeper canonical alignment per hero_service_check_fix/SKILL.md:189 is that lab itself (not the CLI's --start flag) should be the lifecycle owner. This issue addresses the legacy CLI path while it exists. Separate META could track migrating the whole workspace off the CLI lifecycle pattern.

Refs: hero_proc#102, hero_proc#105.

Surfaced by s110 hero_logic D-10 closure ([hero_proc#102 D-10 sweep](https://forge.ourworld.tf/lhumina_code/hero_proc/issues/102)). ## Problem `hero_service::HeroService::rpc()` and `::ui()` only seed `RUST_LOG=info` in `env_vars`. When the CLI invokes `HeroServices::start()`, the resulting `ActionSpec` carries only `RUST_LOG`. hero_proc spawns daemons with **clean env**, so under hero_lib `30a0b34e`'s strict PATH_ROOT regime, every spawned daemon panics at `herolib_core::base::paths::path_root()` (`crates/core/src/base/paths.rs:38`). This is s109 lesson #19, restated for the `hero_service` abstraction layer. ## Current workaround Each CLI that uses `HeroServices` has to thread env vars manually: ```rust fn forward_env(mut service: HeroService) -> HeroService { for key in ["PATH_ROOT", "PATH_VAR", "PATH_BUILD", "PATH_CODE", "HERO_SOCKET_DIR"] { if let Ok(v) = std::env::var(key) && !v.is_empty() { service = service.env(key, v); } } service } let services = HeroServices::new(SERVICE_NAME) .add(forward_env(HeroService::rpc("hero_logic_server"))) .add(forward_env(HeroService::ui("hero_logic_admin").socket_name("admin.sock"))); ``` Live example: [hero_logic ba74b2b](https://forge.ourworld.tf/lhumina_code/hero_logic/commit/ba74b2b) `crates/hero_logic/src/main.rs`. ## Proposed fix Move the forwarding into `HeroService::rpc()` and `HeroService::ui()` directly — default-seed PATH_ROOT/PATH_VAR/PATH_BUILD/PATH_CODE/HERO_SOCKET_DIR from the parent process env when set and non-empty. The few CLI consumers each strip ~6 lines, and every future consumer benefits without thinking about it. Alternative: a single method `.forward_hero_env()` on `HeroService` that does the loop, opted into by callers. ## Affected consumers (as of 2026-05-17) - hero_logic (s110, this commit) - s109 hero_indexer uses a different shape (raw `hero_proc_sdk::ActionSpec` with its own `forward_env_if_set` helper) — would also benefit if it migrated to `HeroServices` - Future D-10 sweeps with the CLI lifecycle pattern ## Out of scope The deeper canonical alignment per `hero_service_check_fix/SKILL.md:189` is that lab itself (not the CLI's `--start` flag) should be the lifecycle owner. This issue addresses the legacy CLI path while it exists. Separate META could track migrating the whole workspace off the CLI lifecycle pattern. Refs: [hero_proc#102](https://forge.ourworld.tf/lhumina_code/hero_proc/issues/102), [hero_proc#105](https://forge.ourworld.tf/lhumina_code/hero_proc/issues/105).