Consolidate herolib_core::base + hero_lifecycle into one crate; replace HeroLifecycle/HeroRpcServer/HeroUiServer with ServiceRpcServer/ServiceAdminServer builders #142

Open
opened 2026-05-26 08:18:45 +00:00 by timur · 2 comments
Owner

Background

Today Hero service authoring is split across three places:

  • herolib_core::base (hero_lib) — ServiceToml schema, validate_service_toml, handle_info_flag, print_startup_banner, prepare_sockets, socket/path resolution, service_base!() macro.
  • hero_lifecycle (this repo) — HeroLifecycle (hero_proc install/start/stop), HeroRpcServer/HeroUiServer (axum UDS serve helpers), serve_unix, shutdown_signal, mandatory-endpoint injection, test::* harness, and a hero_lifecycle inspection CLI binary.
  • lab (hero_skills/crates/lab) — calls <bin> --info, parses service.toml, registers with hero_proc, runs install/start/stop. This is the canonical lifecycle owner per hero_service_implementation.md and hero_service_refactor.md.

The hero_service_refactor.md skill explicitly names HeroLifecycle, HeroRpcServer, and HeroUiServer as legacy patterns to retire. lab already owns lifecycle, so the per-binary --start/--stop/install flags in hero_lifecycle are duplicating lab's job.

At the same time, herolib_core::base only covers startup primitives. Every Hero service still hand-rolls the UDS accept loop, socket permissions, shutdown signal handling, mandatory /health + /openrpc.json + /.well-known/heroservice.json endpoints, and per-test integration harness. That boilerplate is ~60 lines per service today and easy to drift on.

Goal

Merge everything service-authoring-related into one crate, hero_lifecycle, structured around two clean builder entry points (ServiceRpcServer, ServiceAdminServer). Drop the hero_proc_sdk dependency entirely (lab owns proc registration). Update the canonical skills to point at the consolidated crate.

Crate structure after the change

hero_lifecycle becomes the single "Hero service authoring + inspection kit":

hero_lifecycle/
  src/
    base.rs       (moved from herolib_core::base — ServiceToml + closed enums,
                   validate, handle_info, banner, prepare_sockets, path/socket
                   resolution. Renames `extras` → `metadata`)
    serve.rs      (serve_unix accept loop + shutdown_signal + 0o770 helper)
    rpc.rs        (ServiceRpcServer + PreparedRpcServer + mandatory_router)
    admin.rs      (ServiceAdminServer + PreparedAdminServer)
    test.rs       (http_get/post, assert_health, assert_openrpc_valid,
                   assert_rpc_discover, assert_standard_endpoints,
                   ServiceHarness, RunningHarness — kept)
    lib.rs        (re-exports, service_base!() macro)

No hero_proc_sdk dependency. No lifecycle.rs, no service.rs, no cli.rs, no bin/main.rs.

Builder API

Typestate two-stage builder. .build() runs the pre-startup chain (validate → handle-info → banner) and exits the process on --info. .serve(router) consumes the prepared server, does prepare_sockets + bind + 0o770 + accept + shutdown + cleanup.

RPC backend (kind = "server")

use hero_lifecycle::{service_base, ServiceRpcServer};

service_base!();  // SERVICE_TOML + BUILD_NR consts

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    ServiceRpcServer::new("hero_x_server", SERVICE_TOML, BUILD_NR)
        .openrpc(include_str!("../openrpc.json"))
        .metadata(&[("db", db_path.as_str())])
        .build()                       // validate + handle --info (exits) + banner
        .serve(build_router().await?)  // prepare_sockets + bind + accept + shutdown
        .await
}

ServiceRpcServer internally merges mandatory_router(name, spec) (the three required endpoints) over the user router before binding.

Admin dashboard (kind = "admin")

Same shape, no openrpc() (admin doesn't expose JSON-RPC), no mandatory-endpoint injection:

use hero_lifecycle::{service_base, ServiceAdminServer};

service_base!();

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    ServiceAdminServer::new("hero_x_admin", SERVICE_TOML, BUILD_NR)
        .metadata(&[("templates", tmpl_dir.as_str())])
        .build()
        .serve(build_admin_router())
        .await
}

Name is ServiceAdminServer, not ServiceUiServer — aligns with the canonical kind = "admin" closed-enum value. _ui is legacy per hero_service_refactor.md.

Raw escape hatch

Services that need raw control (e.g. hero_proc_server running under screen) call the base primitives directly — they stay public:

use hero_lifecycle::base;
use hero_lifecycle::serve::{serve_unix, shutdown_signal};

base::validate_service_toml(SERVICE_TOML);
base::handle_info_flag(SERVICE_TOML);
base::print_startup_banner("name", SERVICE_TOML, BUILD_NR, &[]);
let socks = base::prepare_sockets("name", SERVICE_TOML);
// ... custom bind / serve ...

Cargo dependency aliasing for readability

Crate name stays hero_lifecycle (per naming_convention skill — hero_* prefix is mandatory for top-level packages). Consumer crates can alias the import for cleaner reads:

[dependencies]
lifecycle = { package = "hero_lifecycle", git = "https://forge.ourworld.tf/lhumina_code/hero_rpc.git", branch = "development" }

Then use lifecycle::ServiceRpcServer; everywhere.

Inspection commands → lab

The hero_lifecycle binary (list, health, test, discover, run, stop, status) is deleted. Cross-service introspection moves into lab:

  • lab service --list — walk $HERO_SOCKET_DIR, print every *.sock
  • lab service --health [name|--all] — GET /health on one or all
  • lab service --test [name|--all]assert_standard_endpoints on one or all
  • lab service --discover <name> — pretty-print /openrpc.json

Lab depends on hero_lifecycle (just the library) and calls into hero_lifecycle::test::* for the assertions. No duplicated implementation.

run/stop/status are already in lab as --start/--stop/--status. No need to re-add.

Migration plan (file-by-file)

hero_lib side

  • Move crates/core/src/base/{mod.rs, service.rs, README.md} into hero_rpc/crates/hero_lifecycle/src/base.rs (+ split as needed).
  • Move related path helpers (path_root, path_var, path_code, hero_socket_dir, hero_bin, hero_var_dir) and the service_base!() macro into the same hero_lifecycle crate.
  • Rename print_startup_banner's extras: &[(&str, &str)] parameter to metadata: &[(&str, &str)].
  • Leave a #[deprecated] shim in herolib_core::base that re-exports from hero_lifecycle::base so the migration can be staged across consumer repos. Targeted for one release cycle, then deleted.

hero_rpc/crates/hero_lifecycle/ side

  • Delete lifecycle.rs (HeroLifecycle, CompanionBinary).
  • Delete service.rs (HeroService, HeroServices, ServiceKind).
  • Delete cli.rs (LifecycleCommand).
  • Delete src/bin/main.rs (binary; functionality moves to lab).
  • Strip hero_proc_sdk from Cargo.toml.
  • Trim hero_server.rs into rpc.rs + admin.rs + serve.rs: keep serve_unix, shutdown_signal, socket-perms 0o770 setup, mandatory-endpoint router builder. Drop .run()/.run_simple()/.run_raw() wrappers (lifecycle branching gone).
  • Keep all of test.rs as-is — it's pure library, no proc dep.
  • Update service.toml for the crate to drop the [[binaries]] entry (no more binary). Or remove service.toml entirely if no kind = "cli" left.

Generator side (crates/generator/src/build/scaffold.rs)

  • Replace the use hero_lifecycle::HeroLifecycle; + HeroLifecycle::new(...).start() + --start/--stop arg sniffing in generate_server_main_rs (currently lines ~1103–1142) with the new ServiceRpcServer::new(...).build().serve(...) pattern.
  • Same for admin scaffold path (use ServiceAdminServer).
  • Drop the hero_lifecycle = { ... } Cargo dep emission since hero_rpc_osis::rpc::bootstrap::MultiDomainBuilder already covers the canonical scaffolded server. Decide: do we keep MultiDomainBuilder::production or fold it into ServiceRpcServer? (Both do the same thing — the former is OSIS-specific glue, the latter is the new generic primitive. Likely keep MultiDomainBuilder calling into ServiceRpcServer internally so OSIS-specific orchestration stays in OSIS.)

Downstream consumers

Not in scope for this issue — each downstream repo migrates on its own cadence under hero_service_refactor.md. Confirmed consumers (grep -r hero_lifecycle across lhumina_code):

  • hero_proxy, hero_osis, hero_compute, hero_os, hero_router, hero_voice, hero_service (template), hero_inspector, hero_index_ui_old, hero_indexer_ui, hero_auth (+ a few archived copies).

The #[deprecated] re-export shim in herolib_core::base means existing builds keep working while each repo gets its own refactor PR.

Skill / spec updates needed

Update after the code lands:

  1. hero_service_implementation — replace the hand-wired main.rs patterns with the new ServiceRpcServer/ServiceAdminServer builder examples.
  2. hero_service_refactor — add a step pointing at ServiceRpcServer/ServiceAdminServer as the replacement for HeroLifecycle/HeroRpcServer/HeroUiServer.
  3. hero_service_check — audit rules update to accept the new builder pattern; flag any service still using HeroLifecycle directly.
  4. hero_proc_service_singlebin — currently documents per-binary --start/--stop flags. Rewrite to say binary runs plain; lab owns install/start/stop.
  5. hero_proc_service_selfstart — same: multi-component registration moves to lab parsing each binary's --info.
  6. hero_service_scaffold — confirm the scaffolder emits the new pattern.

Alignment check (already validated against pulled hero_skills@development)

Skill / spec Status
naming_convention ✓ — hero_lifecycle keeps hero_* prefix; ServiceRpcServer/ServiceAdminServer mirror kind = "server"/"admin".
hero_service_implementation ✓ — builder internally calls every required base::* fn in the mandated order; nothing hand-rolled.
hero_service_check ✓ — no fn print_info_json / fn print_startup_info / hand-rolled socket cleanup in scaffolded main.rs.
hero_service_toml_info ✓ — ServiceToml schema preserved verbatim; closed enums unchanged.
hero_sockets ✓ — prepare_sockets honours $HERO_SOCKET_DIR layout; admin gets admin.sock.
rust_shutdownsignals ✓ — SIGINT + SIGTERM in the built-in serve().
lab / SPECS_PROC_START ✓ — binaries emit canonical service.toml via --info; lab parses and registers with hero_proc; no --start/--stop flags on the binary.

Acceptance criteria

  • hero_lifecycle crate contains the consolidated module set (base, serve, rpc, admin, test).
  • hero_lifecycle/Cargo.toml no longer depends on hero_proc_sdk.
  • crates/hero_lifecycle/src/{lifecycle.rs, service.rs, cli.rs, bin/main.rs} deleted.
  • herolib_core::base ships as a #[deprecated] re-export shim from hero_lifecycle::base (companion PR in hero_lib).
  • crates/generator/src/build/scaffold.rs emits ServiceRpcServer / ServiceAdminServer builders, no HeroLifecycle::new(...).
  • lab service --list / --health / --test / --discover work; the hero_lifecycle binary is gone.
  • cargo build --workspace is clean.
  • At least one downstream service (e.g. scaffolded test service) builds + runs against the new API end-to-end via lab service <name> --install && lab service <name> --start.
  • Skills updated per the list above.

Out of scope

  • Migrating each downstream consumer (hero_osis, hero_compute, hero_os, etc.) — those are individual PRs under hero_service_refactor.
  • Removing the #[deprecated] shim in herolib_core::base — happens one release cycle after this lands.
## Background Today Hero service authoring is split across three places: - **`herolib_core::base`** (`hero_lib`) — `ServiceToml` schema, `validate_service_toml`, `handle_info_flag`, `print_startup_banner`, `prepare_sockets`, socket/path resolution, `service_base!()` macro. - **`hero_lifecycle`** (this repo) — `HeroLifecycle` (hero_proc install/start/stop), `HeroRpcServer`/`HeroUiServer` (axum UDS serve helpers), `serve_unix`, `shutdown_signal`, mandatory-endpoint injection, `test::*` harness, and a `hero_lifecycle` inspection CLI binary. - **`lab`** (`hero_skills/crates/lab`) — calls `<bin> --info`, parses `service.toml`, registers with hero_proc, runs install/start/stop. This is the canonical lifecycle owner per `hero_service_implementation.md` and `hero_service_refactor.md`. The `hero_service_refactor.md` skill explicitly names `HeroLifecycle`, `HeroRpcServer`, and `HeroUiServer` as **legacy patterns to retire**. `lab` already owns lifecycle, so the per-binary `--start`/`--stop`/`install` flags in `hero_lifecycle` are duplicating lab's job. At the same time, `herolib_core::base` only covers startup *primitives*. Every Hero service still hand-rolls the UDS accept loop, socket permissions, shutdown signal handling, mandatory `/health` + `/openrpc.json` + `/.well-known/heroservice.json` endpoints, and per-test integration harness. That boilerplate is ~60 lines per service today and easy to drift on. ## Goal Merge everything service-authoring-related into **one crate, `hero_lifecycle`**, structured around two clean builder entry points (`ServiceRpcServer`, `ServiceAdminServer`). Drop the `hero_proc_sdk` dependency entirely (lab owns proc registration). Update the canonical skills to point at the consolidated crate. ## Crate structure after the change `hero_lifecycle` becomes the single "Hero service authoring + inspection kit": ``` hero_lifecycle/ src/ base.rs (moved from herolib_core::base — ServiceToml + closed enums, validate, handle_info, banner, prepare_sockets, path/socket resolution. Renames `extras` → `metadata`) serve.rs (serve_unix accept loop + shutdown_signal + 0o770 helper) rpc.rs (ServiceRpcServer + PreparedRpcServer + mandatory_router) admin.rs (ServiceAdminServer + PreparedAdminServer) test.rs (http_get/post, assert_health, assert_openrpc_valid, assert_rpc_discover, assert_standard_endpoints, ServiceHarness, RunningHarness — kept) lib.rs (re-exports, service_base!() macro) ``` No `hero_proc_sdk` dependency. No `lifecycle.rs`, no `service.rs`, no `cli.rs`, no `bin/main.rs`. ## Builder API Typestate two-stage builder. `.build()` runs the pre-startup chain (validate → handle-info → banner) and exits the process on `--info`. `.serve(router)` consumes the prepared server, does prepare_sockets + bind + 0o770 + accept + shutdown + cleanup. ### RPC backend (`kind = "server"`) ```rust use hero_lifecycle::{service_base, ServiceRpcServer}; service_base!(); // SERVICE_TOML + BUILD_NR consts #[tokio::main] async fn main() -> anyhow::Result<()> { ServiceRpcServer::new("hero_x_server", SERVICE_TOML, BUILD_NR) .openrpc(include_str!("../openrpc.json")) .metadata(&[("db", db_path.as_str())]) .build() // validate + handle --info (exits) + banner .serve(build_router().await?) // prepare_sockets + bind + accept + shutdown .await } ``` `ServiceRpcServer` internally merges `mandatory_router(name, spec)` (the three required endpoints) over the user router before binding. ### Admin dashboard (`kind = "admin"`) Same shape, no `openrpc()` (admin doesn't expose JSON-RPC), no mandatory-endpoint injection: ```rust use hero_lifecycle::{service_base, ServiceAdminServer}; service_base!(); #[tokio::main] async fn main() -> anyhow::Result<()> { ServiceAdminServer::new("hero_x_admin", SERVICE_TOML, BUILD_NR) .metadata(&[("templates", tmpl_dir.as_str())]) .build() .serve(build_admin_router()) .await } ``` Name is `ServiceAdminServer`, not `ServiceUiServer` — aligns with the canonical `kind = "admin"` closed-enum value. `_ui` is legacy per `hero_service_refactor.md`. ### Raw escape hatch Services that need raw control (e.g. `hero_proc_server` running under screen) call the base primitives directly — they stay public: ```rust use hero_lifecycle::base; use hero_lifecycle::serve::{serve_unix, shutdown_signal}; base::validate_service_toml(SERVICE_TOML); base::handle_info_flag(SERVICE_TOML); base::print_startup_banner("name", SERVICE_TOML, BUILD_NR, &[]); let socks = base::prepare_sockets("name", SERVICE_TOML); // ... custom bind / serve ... ``` ## Cargo dependency aliasing for readability Crate name stays `hero_lifecycle` (per `naming_convention` skill — `hero_*` prefix is mandatory for top-level packages). Consumer crates can alias the import for cleaner reads: ```toml [dependencies] lifecycle = { package = "hero_lifecycle", git = "https://forge.ourworld.tf/lhumina_code/hero_rpc.git", branch = "development" } ``` Then `use lifecycle::ServiceRpcServer;` everywhere. ## Inspection commands → `lab` The `hero_lifecycle` binary (`list`, `health`, `test`, `discover`, `run`, `stop`, `status`) is deleted. Cross-service introspection moves into `lab`: - `lab service --list` — walk `$HERO_SOCKET_DIR`, print every `*.sock` - `lab service --health [name|--all]` — GET `/health` on one or all - `lab service --test [name|--all]` — `assert_standard_endpoints` on one or all - `lab service --discover <name>` — pretty-print `/openrpc.json` Lab depends on `hero_lifecycle` (just the library) and calls into `hero_lifecycle::test::*` for the assertions. No duplicated implementation. `run`/`stop`/`status` are already in lab as `--start`/`--stop`/`--status`. No need to re-add. ## Migration plan (file-by-file) ### `hero_lib` side - Move `crates/core/src/base/{mod.rs, service.rs, README.md}` into `hero_rpc/crates/hero_lifecycle/src/base.rs` (+ split as needed). - Move related path helpers (`path_root`, `path_var`, `path_code`, `hero_socket_dir`, `hero_bin`, `hero_var_dir`) and the `service_base!()` macro into the same `hero_lifecycle` crate. - Rename `print_startup_banner`'s `extras: &[(&str, &str)]` parameter to `metadata: &[(&str, &str)]`. - Leave a `#[deprecated]` shim in `herolib_core::base` that re-exports from `hero_lifecycle::base` so the migration can be staged across consumer repos. Targeted for one release cycle, then deleted. ### `hero_rpc/crates/hero_lifecycle/` side - Delete `lifecycle.rs` (HeroLifecycle, CompanionBinary). - Delete `service.rs` (HeroService, HeroServices, ServiceKind). - Delete `cli.rs` (LifecycleCommand). - Delete `src/bin/main.rs` (binary; functionality moves to lab). - Strip `hero_proc_sdk` from `Cargo.toml`. - Trim `hero_server.rs` into `rpc.rs` + `admin.rs` + `serve.rs`: keep `serve_unix`, `shutdown_signal`, socket-perms 0o770 setup, mandatory-endpoint router builder. Drop `.run()`/`.run_simple()`/`.run_raw()` wrappers (lifecycle branching gone). - Keep all of `test.rs` as-is — it's pure library, no proc dep. - Update `service.toml` for the crate to drop the `[[binaries]]` entry (no more binary). Or remove `service.toml` entirely if no `kind = "cli"` left. ### Generator side (`crates/generator/src/build/scaffold.rs`) - Replace the `use hero_lifecycle::HeroLifecycle;` + `HeroLifecycle::new(...).start()` + `--start`/`--stop` arg sniffing in `generate_server_main_rs` (currently lines ~1103–1142) with the new `ServiceRpcServer::new(...).build().serve(...)` pattern. - Same for admin scaffold path (use `ServiceAdminServer`). - Drop the `hero_lifecycle = { ... }` Cargo dep emission since `hero_rpc_osis::rpc::bootstrap::MultiDomainBuilder` already covers the canonical scaffolded server. Decide: do we keep `MultiDomainBuilder::production` or fold it into `ServiceRpcServer`? (Both do the same thing — the former is OSIS-specific glue, the latter is the new generic primitive. Likely keep MultiDomainBuilder calling into ServiceRpcServer internally so OSIS-specific orchestration stays in OSIS.) ### Downstream consumers Not in scope for this issue — each downstream repo migrates on its own cadence under `hero_service_refactor.md`. Confirmed consumers (`grep -r hero_lifecycle` across lhumina_code): - `hero_proxy`, `hero_osis`, `hero_compute`, `hero_os`, `hero_router`, `hero_voice`, `hero_service` (template), `hero_inspector`, `hero_index_ui_old`, `hero_indexer_ui`, `hero_auth` (+ a few archived copies). The `#[deprecated]` re-export shim in `herolib_core::base` means existing builds keep working while each repo gets its own refactor PR. ## Skill / spec updates needed Update after the code lands: 1. **`hero_service_implementation`** — replace the hand-wired `main.rs` patterns with the new `ServiceRpcServer`/`ServiceAdminServer` builder examples. 2. **`hero_service_refactor`** — add a step pointing at `ServiceRpcServer`/`ServiceAdminServer` as the replacement for `HeroLifecycle`/`HeroRpcServer`/`HeroUiServer`. 3. **`hero_service_check`** — audit rules update to accept the new builder pattern; flag any service still using `HeroLifecycle` directly. 4. **`hero_proc_service_singlebin`** — currently documents per-binary `--start`/`--stop` flags. Rewrite to say binary runs plain; lab owns install/start/stop. 5. **`hero_proc_service_selfstart`** — same: multi-component registration moves to lab parsing each binary's `--info`. 6. **`hero_service_scaffold`** — confirm the scaffolder emits the new pattern. ## Alignment check (already validated against pulled `hero_skills@development`) | Skill / spec | Status | |---|---| | `naming_convention` | ✓ — `hero_lifecycle` keeps `hero_*` prefix; `ServiceRpcServer`/`ServiceAdminServer` mirror `kind = "server"`/`"admin"`. | | `hero_service_implementation` | ✓ — builder internally calls every required `base::*` fn in the mandated order; nothing hand-rolled. | | `hero_service_check` | ✓ — no `fn print_info_json` / `fn print_startup_info` / hand-rolled socket cleanup in scaffolded main.rs. | | `hero_service_toml_info` | ✓ — `ServiceToml` schema preserved verbatim; closed enums unchanged. | | `hero_sockets` | ✓ — `prepare_sockets` honours `$HERO_SOCKET_DIR` layout; admin gets `admin.sock`. | | `rust_shutdownsignals` | ✓ — SIGINT + SIGTERM in the built-in `serve()`. | | `lab` / `SPECS_PROC_START` | ✓ — binaries emit canonical `service.toml` via `--info`; lab parses and registers with hero_proc; no `--start`/`--stop` flags on the binary. | ## Acceptance criteria - [ ] `hero_lifecycle` crate contains the consolidated module set (`base`, `serve`, `rpc`, `admin`, `test`). - [ ] `hero_lifecycle/Cargo.toml` no longer depends on `hero_proc_sdk`. - [ ] `crates/hero_lifecycle/src/{lifecycle.rs, service.rs, cli.rs, bin/main.rs}` deleted. - [ ] `herolib_core::base` ships as a `#[deprecated]` re-export shim from `hero_lifecycle::base` (companion PR in `hero_lib`). - [ ] `crates/generator/src/build/scaffold.rs` emits `ServiceRpcServer` / `ServiceAdminServer` builders, no `HeroLifecycle::new(...)`. - [ ] `lab service --list / --health / --test / --discover` work; the `hero_lifecycle` binary is gone. - [ ] `cargo build --workspace` is clean. - [ ] At least one downstream service (e.g. scaffolded test service) builds + runs against the new API end-to-end via `lab service <name> --install && lab service <name> --start`. - [ ] Skills updated per the list above. ## Out of scope - Migrating each downstream consumer (`hero_osis`, `hero_compute`, `hero_os`, etc.) — those are individual PRs under `hero_service_refactor`. - Removing the `#[deprecated]` shim in `herolib_core::base` — happens one release cycle after this lands.
Author
Owner

Read this against the just-pulled hero_skills@development. Two notes after that pass:

1. admin.sock settled (no change needed)

The alignment-check line "admin gets admin.sock" is correct. Verified across hero_sockets.md §3.2, hero_service_refactor.md (lines 31/109/129), hero_service_check.md, hero_service_toml_info.md:300, naming_convention.md, hero_router.md, hero_libs.md, hero_voice_widget.md, hero_ui_dashboard_implementation.md, hero_ui_openrpc_proxy.md, hero_browser.md, hs_service_scaffold.md — every one uses admin.sock. Only ui.sock references are explicit legacy-migration callouts (hero_service_refactor.md:31). Outlier worth a cleanup pass: rust_hero_repo_create.md:188 still mentions ui.sock.

2. Env-var spelling glitch in hero_sockets.md

hero_sockets.md §1 documents the root as $PATH_VAR/sockets but the Rust snippet reads std::env::var("PATH_VAR_sockets") — that env var name doesn't appear anywhere in the implementation. The actual resolver hierarchy (per lab/service/ephemeral.rs:178-187 and herolib_core::base) is PATH_ROOTPATH_VARHERO_SOCKET_DIR, with lab clearing all four when it pivots into an ephemeral scratch dir.

Since this PR makes hero_lifecycle::base the single owner of the resolver, it's the right moment to pick a canonical env-var name (recommend PATH_VAR to match the §1 path) and update both code and skill in lockstep.

3. Suggested addition: peer_socket_path helper

The new ServiceRpcServer/ServiceAdminServer builders fix the bind half (everyone routes through prepare_sockets, so a binary can no longer hand-roll a socket directory). They don't cover the connect half — e.g. a UI binary connecting to its sibling RPC server.

Today the scaffolder emits crates/generator/src/build/scaffold.rs:3313-3314:

let sock = herolib_core::base::resolve_socket_dir()
    .join("{service_name}_server/rpc.sock");

Which is wrong: the server's service.toml declares {svc}/rpc.sock, not {svc}_server/rpc.sock (the _server suffix violates hero_sockets.md §2). The UI as scaffolded today doesn't connect.

Adding a single helper next to prepare_sockets:

/// Resolve a peer service's socket path. `svc` is the **service name**
/// (no `_server`/`_admin` suffix); `socket_type` is `"rpc"`, `"admin"`,
/// `"rest"`, etc.
pub fn peer_socket_path(svc: &str, socket_type: &str) -> std::path::PathBuf {
    resolve_socket_dir().join(svc).join(format!("{socket_type}.sock"))
}

…closes the loop. After this lands, the scaffolded AppState::from_env becomes peer_socket_path(SERVICE_NAME, "rpc") and consumer code never spells a directory name again. Worth pulling into #142 since hero_lifecycle::base is exactly the right home for it.

A companion follow-up issue (just opened) covers the orthogonal scaffolder / generator cleanups: the broken AppState::from_env connect, the hardcoded /tmp/herozero_*_e2e data dir in generate/e2e.rs, and the legacy hero_indexer_server.sock probe fallback in find_tests_emit.rs + benches/index_perf.rs. None of those overlap with the consolidation work — they're just stale.

No blocking concerns on the consolidation otherwise. Looking forward to it.

Read this against the just-pulled `hero_skills@development`. Two notes after that pass: ### 1. `admin.sock` settled (no change needed) The alignment-check line _"admin gets `admin.sock`"_ is correct. Verified across `hero_sockets.md` §3.2, `hero_service_refactor.md` (lines 31/109/129), `hero_service_check.md`, `hero_service_toml_info.md:300`, `naming_convention.md`, `hero_router.md`, `hero_libs.md`, `hero_voice_widget.md`, `hero_ui_dashboard_implementation.md`, `hero_ui_openrpc_proxy.md`, `hero_browser.md`, `hs_service_scaffold.md` — every one uses `admin.sock`. Only `ui.sock` references are explicit legacy-migration callouts (`hero_service_refactor.md:31`). Outlier worth a cleanup pass: `rust_hero_repo_create.md:188` still mentions `ui.sock`. ### 2. Env-var spelling glitch in `hero_sockets.md` `hero_sockets.md` §1 documents the root as `$PATH_VAR/sockets` but the Rust snippet reads `std::env::var("PATH_VAR_sockets")` — that env var name doesn't appear anywhere in the implementation. The actual resolver hierarchy (per `lab/service/ephemeral.rs:178-187` and `herolib_core::base`) is `PATH_ROOT` → `PATH_VAR` → `HERO_SOCKET_DIR`, with lab clearing all four when it pivots into an ephemeral scratch dir. Since this PR makes `hero_lifecycle::base` the single owner of the resolver, it's the right moment to pick a canonical env-var name (recommend `PATH_VAR` to match the §1 path) and update both code and skill in lockstep. ### 3. Suggested addition: `peer_socket_path` helper The new `ServiceRpcServer`/`ServiceAdminServer` builders fix the *bind* half (everyone routes through `prepare_sockets`, so a binary can no longer hand-roll a socket directory). They don't cover the *connect* half — e.g. a UI binary connecting to its sibling RPC server. Today the scaffolder emits `crates/generator/src/build/scaffold.rs:3313-3314`: ```rust let sock = herolib_core::base::resolve_socket_dir() .join("{service_name}_server/rpc.sock"); ``` Which is wrong: the server's `service.toml` declares `{svc}/rpc.sock`, not `{svc}_server/rpc.sock` (the `_server` suffix violates `hero_sockets.md` §2). The UI as scaffolded today doesn't connect. Adding a single helper next to `prepare_sockets`: ```rust /// Resolve a peer service's socket path. `svc` is the **service name** /// (no `_server`/`_admin` suffix); `socket_type` is `"rpc"`, `"admin"`, /// `"rest"`, etc. pub fn peer_socket_path(svc: &str, socket_type: &str) -> std::path::PathBuf { resolve_socket_dir().join(svc).join(format!("{socket_type}.sock")) } ``` …closes the loop. After this lands, the scaffolded `AppState::from_env` becomes `peer_socket_path(SERVICE_NAME, "rpc")` and consumer code never spells a directory name again. Worth pulling into #142 since `hero_lifecycle::base` is exactly the right home for it. A companion follow-up issue (just opened) covers the orthogonal scaffolder / generator cleanups: the broken `AppState::from_env` connect, the hardcoded `/tmp/herozero_*_e2e` data dir in `generate/e2e.rs`, and the legacy `hero_indexer_server.sock` probe fallback in `find_tests_emit.rs` + `benches/index_perf.rs`. None of those overlap with the consolidation work — they're just stale. No blocking concerns on the consolidation otherwise. Looking forward to it.
Author
Owner

Landed on development as 50d132f.

Out of scope for the followup:

  • herolib_core::base deprecation shim (companion PR in hero_lib) — deferred to avoid cross-workspace cargo-graph duplication (workspace-local hero_lifecycle vs git-fetched copy via herolib_core).
  • Downstream consumer migrations (hero_proxy, hero_osis, hero_compute, hero_os, hero_router, hero_voice, hero_service template, hero_inspector, hero_index_ui_old, hero_indexer_ui, hero_auth, hero_compute_manager) — individual PRs under hero_service_refactor.
  • Fold hero_lifecycle inspection CLI into lab (lab service --list / --health / --test / --discover).
  • Skill updates (hero_service_implementation, hero_service_refactor, hero_service_check, hero_proc_service_singlebin, hero_proc_service_selfstart, hero_service_scaffold) — point at the new ServiceRpcServer / ServiceAdminServer pattern.
  • Scaffolder / generator cleanups from the review comment (broken AppState::from_env connect, hardcoded /tmp/herozero_*_e2e data dir, legacy hero_indexer_server.sock probe) — separate follow-up issue per the comment.

Incorporated from the review comment:

  • resolve_socket_dir cascade widened: HERO_SOCKET_DIR -> PATH_VAR -> PATH_ROOT -> $HOME/hero/var/sockets (matches hero_sockets.md §1).
  • New hero_lifecycle::base::peer_socket_path(service_name, socket_type) helper for the connect side, paired with prepare_sockets on the bind side.
Landed on development as 50d132f. **Out of scope for the followup**: - herolib_core::base deprecation shim (companion PR in hero_lib) — deferred to avoid cross-workspace cargo-graph duplication (workspace-local hero_lifecycle vs git-fetched copy via herolib_core). - Downstream consumer migrations (hero_proxy, hero_osis, hero_compute, hero_os, hero_router, hero_voice, hero_service template, hero_inspector, hero_index_ui_old, hero_indexer_ui, hero_auth, hero_compute_manager) — individual PRs under hero_service_refactor. - Fold hero_lifecycle inspection CLI into lab (lab service --list / --health / --test / --discover). - Skill updates (hero_service_implementation, hero_service_refactor, hero_service_check, hero_proc_service_singlebin, hero_proc_service_selfstart, hero_service_scaffold) — point at the new ServiceRpcServer / ServiceAdminServer pattern. - Scaffolder / generator cleanups from the review comment (broken AppState::from_env connect, hardcoded /tmp/herozero_*_e2e data dir, legacy hero_indexer_server.sock probe) — separate follow-up issue per the comment. **Incorporated from the review comment**: - resolve_socket_dir cascade widened: HERO_SOCKET_DIR -> PATH_VAR -> PATH_ROOT -> $HOME/hero/var/sockets (matches hero_sockets.md §1). - New hero_lifecycle::base::peer_socket_path(service_name, socket_type) helper for the connect side, paired with prepare_sockets on the bind side.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_rpc#142
No description provided.