Scaffolder/generator: remove hardcoded socket and data paths; drop legacy indexer probe fallback #143

New issue

Closed

opened 2026-05-26 08:56:19 +00:00 by timur · 1 comment

timur commented

2026-05-26 08:56:19 +00:00

Owner

Context

While auditing socket-path resolution against the hero_sockets spec and lab's --start --ephemeral pivot (lab clears every env var except PATH_ROOT, so any path the generated code hardcodes silently escapes the scratch dir and connects to the contributor's real ~/hero/var/), a handful of stale path constructions surfaced in the generator and the scaffolded output. None of them are addressed by #142 (which only owns the bind half via ServiceRpcServer/ServiceAdminServer). They are independent cleanups.

Following the principle articulated by #142 (service.toml is the only path declaration; consumers never spell directory names): every site in the generator that constructs a socket or data path by string-concat should be replaced by a typed helper that reads the canonical resolver chain (PATH_ROOT → PATH_VAR → HERO_SOCKET_DIR → ~/hero/var/sockets).

Goals

crates/generator/src/build/scaffold.rs:3313-3314 — scaffolded AppState::from_env currently does resolve_socket_dir().join("{service_name}_server/rpc.sock"). The server's own service.toml declares {svc}/rpc.sock (no _server suffix per hero_sockets.md §2). Replace with a peer_socket_path(SERVICE_NAME, "rpc") call against the helper added in #142 (or, if #142 doesn't add it, with resolve_socket_dir().join(SERVICE_NAME).join("rpc.sock")). Scaffolded UI as it stands today cannot connect to the scaffolded server.
crates/generator/src/generate/e2e.rs:99 — generated client_server.rs example sets let data_dir = "/tmp/herozero_{}_e2e";. Hardcoded /tmp defeats lab's PATH_ROOT pivot and any HERO_DATA_DIR override. Resolve via herolib_core::base::hero_var_dir().join("osisdb").join(...) (or whichever helper survives the #142 consolidation) so the example respects the same env contract as production.
crates/generator/src/build/find_tests_emit.rs:160-165 — emitted indexer_reachable() helper probes <base>/hero_indexer/rpc.sock then falls back to legacy flat <base>/hero_indexer_server.sock. The legacy path violates hero_sockets.md §2 (no flat-file sockets directly under $PATH_VAR/sockets/) and is no longer produced by any current hero_indexer build. Drop the fallback; canonical path only.
benches/benches/index_perf.rs:537-551 — same legacy hero_indexer_server.sock fallback in indexer_socket_path(). Drop the fallback symmetrically with the find-tests emitter so a single canonical layout is enforced everywhere.
crates/generator/src/build/scaffold.rs doc/string drift (lines 1666, 1741, 1790, 2611, 2674, 3290, 3311, 4057, 4059) — README cells and rustdoc comments still reference {name}_server/rpc.sock, {name}_admin/admin.sock, {name}_web/web.sock. Sweep to the canonical {name}/rpc.sock / {name}/admin.sock / {name}/web_<name>.sock shape. (Doc-only; no behaviour change.)
Verify the scaffolder snapshot tests still pass after the doc sweep; add a unit test that asserts AppState::from_env connects to <svc>/rpc.sock (not <svc>_server/rpc.sock) so the regression can't return.

Out of scope

Anything touching hero_lifecycle / ServiceRpcServer / ServiceAdminServer — that's #142.
The hero_admin_lib::admin_socket_path helper which today computes {name}_admin/admin.sock — #142's ServiceAdminServer.serve() routing through prepare_sockets makes that helper obsolete; deletion happens in #142's PR.
Picking a canonical env-var name (PATH_VAR vs HERO_SOCKET_DIR) — surfaced as a comment on #142.

## Context While auditing socket-path resolution against the `hero_sockets` spec and lab's `--start --ephemeral` pivot (lab clears every env var except `PATH_ROOT`, so any path the generated code hardcodes silently escapes the scratch dir and connects to the contributor's real `~/hero/var/`), a handful of stale path constructions surfaced in the generator and the scaffolded output. None of them are addressed by #142 (which only owns the *bind* half via `ServiceRpcServer`/`ServiceAdminServer`). They are independent cleanups. Following the principle articulated by #142 (`service.toml` is the only path declaration; consumers never spell directory names): every site in the generator that constructs a socket or data path by string-concat should be replaced by a typed helper that reads the canonical resolver chain (`PATH_ROOT` → `PATH_VAR` → `HERO_SOCKET_DIR` → `~/hero/var/sockets`). ## Goals - `crates/generator/src/build/scaffold.rs:3313-3314` — scaffolded `AppState::from_env` currently does `resolve_socket_dir().join("{service_name}_server/rpc.sock")`. The server's own `service.toml` declares `{svc}/rpc.sock` (no `_server` suffix per `hero_sockets.md` §2). Replace with a `peer_socket_path(SERVICE_NAME, "rpc")` call against the helper added in #142 (or, if #142 doesn't add it, with `resolve_socket_dir().join(SERVICE_NAME).join("rpc.sock")`). Scaffolded UI as it stands today cannot connect to the scaffolded server. - `crates/generator/src/generate/e2e.rs:99` — generated `client_server.rs` example sets `let data_dir = "/tmp/herozero_{}_e2e";`. Hardcoded `/tmp` defeats lab's `PATH_ROOT` pivot and any `HERO_DATA_DIR` override. Resolve via `herolib_core::base::hero_var_dir().join("osisdb").join(...)` (or whichever helper survives the #142 consolidation) so the example respects the same env contract as production. - `crates/generator/src/build/find_tests_emit.rs:160-165` — emitted `indexer_reachable()` helper probes `<base>/hero_indexer/rpc.sock` then falls back to legacy flat `<base>/hero_indexer_server.sock`. The legacy path violates `hero_sockets.md` §2 (no flat-file sockets directly under `$PATH_VAR/sockets/`) and is no longer produced by any current hero_indexer build. Drop the fallback; canonical path only. - `benches/benches/index_perf.rs:537-551` — same legacy `hero_indexer_server.sock` fallback in `indexer_socket_path()`. Drop the fallback symmetrically with the find-tests emitter so a single canonical layout is enforced everywhere. - `crates/generator/src/build/scaffold.rs` doc/string drift (lines 1666, 1741, 1790, 2611, 2674, 3290, 3311, 4057, 4059) — README cells and rustdoc comments still reference `{name}_server/rpc.sock`, `{name}_admin/admin.sock`, `{name}_web/web.sock`. Sweep to the canonical `{name}/rpc.sock` / `{name}/admin.sock` / `{name}/web_<name>.sock` shape. (Doc-only; no behaviour change.) - Verify the scaffolder snapshot tests still pass after the doc sweep; add a unit test that asserts `AppState::from_env` connects to `<svc>/rpc.sock` (not `<svc>_server/rpc.sock`) so the regression can't return. ### Out of scope - Anything touching `hero_lifecycle` / `ServiceRpcServer` / `ServiceAdminServer` — that's #142. - The `hero_admin_lib::admin_socket_path` helper which today computes `{name}_admin/admin.sock` — #142's `ServiceAdminServer.serve()` routing through `prepare_sockets` makes that helper obsolete; deletion happens in #142's PR. - Picking a canonical env-var name (`PATH_VAR` vs `HERO_SOCKET_DIR`) — surfaced as a comment on #142.

timur referenced this issue from a commit

2026-05-26 09:08:27 +00:00

fix(generator): drop hardcoded socket/data paths and legacy indexer probe fallback

timur referenced this issue

2026-05-26 09:08:41 +00:00

fix(generator): drop hardcoded socket/data paths and legacy indexer fallback #144

timur commented

2026-05-26 09:17:17 +00:00

Author

Owner

Merged in PR #144 (commit a302c6955d) on development.

Changes

scaffold.rs UI AppState::from_env now joins {service_name}/rpc.sock (was {service_name}_server/rpc.sock).
generate/e2e.rs data dir resolved from resolve_socket_dir().parent() instead of /tmp/herozero_*_e2e.
find_tests_emit.rs emitted indexer_reachable() probes only canonical hero_indexer/rpc.sock.
benches/index_perf.rs indexer_socket_path() same legacy fallback dropped.
New regression test test_scaffold_ui_state_uses_canonical_service_socket_path.

151 generator tests pass; cargo build --workspace clean.

Left for follow-up

The admin/web main.rs template docstrings at scaffold.rs:1666 and :1790 ({name}_admin/admin.sock, {name}_web/web.sock) sit inside the emitters that #142 is rewriting — they will be canonicalised in that PR's scaffolder sweep.

The scaffolder itself still emits herolib_core::base::* calls everywhere (consistent with the rest of the scaffolded code). Migrating those to hero_lifecycle::base::* directly is a follow-up that becomes mechanical once the herolib_core::base deprecation re-export shim lands in hero_lib.

Merged in PR #144 (commit a302c6955d) on `development`. ## Changes - `scaffold.rs` UI `AppState::from_env` now joins `{service_name}/rpc.sock` (was `{service_name}_server/rpc.sock`). - `generate/e2e.rs` data dir resolved from `resolve_socket_dir().parent()` instead of `/tmp/herozero_*_e2e`. - `find_tests_emit.rs` emitted `indexer_reachable()` probes only canonical `hero_indexer/rpc.sock`. - `benches/index_perf.rs` `indexer_socket_path()` same legacy fallback dropped. - New regression test `test_scaffold_ui_state_uses_canonical_service_socket_path`. 151 generator tests pass; `cargo build --workspace` clean. ## Left for follow-up The admin/web `main.rs` template docstrings at `scaffold.rs:1666` and `:1790` (`{name}_admin/admin.sock`, `{name}_web/web.sock`) sit inside the emitters that #142 is rewriting — they will be canonicalised in that PR's scaffolder sweep. The scaffolder itself still emits `herolib_core::base::*` calls everywhere (consistent with the rest of the scaffolded code). Migrating those to `hero_lifecycle::base::*` directly is a follow-up that becomes mechanical once the `herolib_core::base` deprecation re-export shim lands in `hero_lib`.

timur closed this issue

2026-05-26 09:17:18 +00:00