hero_rpc#124: MultiDomainBuilder + subprocess-driver tests + Layer 2-4 nu skeletons #126
No reviewers
Labels
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
lhumina_code/hero_rpc!126
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "issue-124-lifecycle-alignment"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes part 1/3 of hero_rpc#124 (lifecycle alignment). Two sibling PRs follow: hero_skills (
lab --ephemeral/--json/--pid/--test) and hero_service (template re-validation).What this PR does
MultiDomainBuilder(internal tohero_rpc_osis) — fluent assembly of oneRpcModulefrom N OSIS domain handlers. Typestate (Production/Ephemeral) so.spawn()returns the right shape per mode.run/run_for_testbecome thin compat wrappers — no consumer breakage.main.rstemplate — collapses the per-domainregister_methodsladder into aMultiDomainBuilder::production()chain. One.with_domain::<…>(…)line per.oschemadomain.tests/src/lib.rstemplate — subprocess-driver shape:Command::new("lab")for spin-up,lab --stop --pidinDrop. Nohero_rpc_osis, nojsonrpsee, no<name>_serverdirect dep. The running lab knows about new domains viaservice.toml, so adding an.oschemarequires zero edits to this file.tests/smoke.nu,tests/api_integration.nu,tests/e2e_<flow>.nu. Each accepts--socket <path>(orHERO_TEST_SOCKETenv), usescurl --unix-socketfor the UDS HTTP path. Smoke covers the four mandatory endpoints; api_integration walks one rootobject CRUD over the wire.docs/testing.md— scaffolded README points contributors at one verb (lab service <name> --test).docs/testing.mdwalks the five-layer pyramid with what each layer catches.developmentwere blockingcargo clippy --workspace -- -D warnings(doc-list-indent, uselessformat!, collapsible if-let). Folded in so this PR lands under the CI gate.Three commits, structured for review:
98c2ce3— MultiDomainBuilder extraction05e1a0b— subprocess-driver tests + nu skeletons + scaffolder regen33e8d60— README + docs/testing.md + clippy fixesHeadline diff stats (hero_recipes_server template)
main.rstests/src/lib.rsThe
tests/src/lib.rsline count went up — the subprocess shape needs error handling aroundCommand::output, JSON parsing, aDropimpl, and a typed client connection that the previous in-process version got for free fromrun_for_test. The win isn't line count; it's drift-free: tests/src/lib.rs is now byte-identical across reruns regardless of how many.oschemadomains exist. The old shape needed a hand-edit to theregister_methodsladder for every new domain.Main.rs collapses by 30% and the per-domain
cfg(feature)block is now a single line per domain.Acceptance (this PR's slice)
Full issue acceptance lands across all three PRs; this PR delivers:
MultiDomainBuilder::production()+MultiDomainBuilder::for_ephemeral()inhero_rpc_osis::rpc::bootstrap, withwith_domain<App>chained registration. Internal tohero_rpc_osis— no public-crate surface added.run_for_test<A>kept as compat wrapper so existingosis_bencheskeeps building.crates/<name>_server/src/main.rsusesMultiDomainBuilder::production()— no inlineregister_methodsladder.tests/src/lib.rsis the subprocess-driver shape — nohero_rpc_osisimport, noMultiDomainBuildermention, no per-domaincfg(feature)block..oschemadomain requires zero edits totests/src/lib.rs.main.rsonly sees a new.with_domain::<…>(…)line after re-scaffolding (verified by regeneratingexamples/recipe_serverend-to-end).tests/with the canonical--socket/HERO_TEST_SOCKETshape.tests/Cargo.tomldropshero_rpc_osis,jsonrpsee, and the<name>_serverpath dep — onlyhero_rpc2(client),<name>_sdk,serde_json,anyhow,tokioremain.cargo test --workspace --lib --binsclean (excluding the pre-existinghero_rpc_osis_benchesbreakage — see Follow-ups).cargo clippy --workspace -- -D warningsclean (excluding the same pre-existing bench breakage).cargo fmt --all -- --checkclean.test_scaffold_tests_lib_rs_is_subprocess_driver,test_scaffold_tests_cargo_toml_drops_bootstrap_deps) and one that pins the nu skeletons (test_scaffold_emits_nushell_skeletons).Deferred to PR2 (hero_skills):
lab service <name> --start --ephemeral --json(the verb the newtests/src/lib.rsshells out to).lab service <name> --stop --pid <pid>flag.lab service <name> --test [layer]verb (the README + docs/testing.md reference this ahead of its merge — clean ordering per the issue's squash-merge plan).hero_service_test_completeskill doc update.Deferred to PR3 (hero_service):
pgrep -f hero_service_serverempty afterlab --test).Decisions taken without confirmation
for_test→for_ephemeralrename. The final issue framing uses "ephemeral" everywhere; the builder constructor matches.tests/src/lib.rsand<server>/src/main.rsswitched towrite_managed(overwrite-on-rerun).write_preserved(the previous default for both files) made the issue's acceptance criterion — "zero hand-edits after re-scaffolding" — impossible to meet. Other scaffolder-emitted files stay preserve-once. NewScaffoldResult::files_updatedfield reports which files were managed.curl --unix-socketinside the nu scripts, not nushell's nativehttp get(which is TCP-only). Thehero_service_test_completeskill calls outcurlas the established escape hatch for the UDS case.examples/recipe_serverregenerated via the new templates. The example workspace isexcluded from the top-level workspace, so hero_rpc CI runscargo checkon it but not the integration tests — the subprocess shape compiles cleanly withoutlabinstalled.development(doc-list-indent inscaffold.rs+rust_osis.rs,format!literals injs_struct.rs, collapsible if-let injs_struct.rs) were blocking the CI gate; fixed in this PR so reviewers don't have to deal with stale red.Follow-ups not done here
osis_benchessubprocess shape (Phase F in the issue). Pendinglab --ephemerallanding in hero_skills PR2; today's benches still callrun_for_testdirectly via the compat wrapper.hero_rpc_osis_benchesmissinggenerated/mod.rs— broken ondevelopmentalready (codegen half-merged from PR #125). Out of scope for #124; should be a follow-up bug.hero_service_test_complete/tests_pyramidskill updates — land in the hero_skills PR alongside thelab --testverb.Test plan
cargo test --workspace --lib --bins --exclude hero_rpc_osis_benchesgreen (446 tests).cargo clippy --workspace --exclude hero_rpc_osis_benches -- -D warningsgreen.cargo fmt --all -- --checkgreen.cargo check --tests --manifest-path examples/recipe_server/Cargo.tomlgreen (subprocess-shape generated tests compile cleanly withoutlabinstalled).examples/recipe_serverproduces a byte-identicaltests/src/lib.rson subsequent runs.cargo test -p hero_recipes_tests— requires hero_skills PR2'slab --ephemeralto be live. Will verify as part of the hero_service PR3 acceptance.Rewrite the scaffolder's `tests/src/lib.rs` template as a subprocess driver around `lab service <name> --start --ephemeral --json`: - `ServiceHandle` holds the connected `hero_rpc2::Client` and the lab-reported pid; `Drop` shells out to `lab --stop --pid` so a panic / failure / early-return can't leak the child. - `spin_up_service()` is byte-identical across reruns — adding a new `.oschema` requires zero edits to the file. The running lab picks up the new domain through `service.toml`. - `tests/Cargo.toml` drops `hero_rpc_osis`, `jsonrpsee`, and the `<name>_server` path dep — the subprocess shape only needs the `hero_rpc2` client, `serde_json`, `anyhow`, `tokio`. Generated `<entity>_e2e.rs` updated to the new ServiceHandle shape (no `TempDir` second-tuple, no `svc.shutdown().await`). Also emit Layer 2-4 nushell skeletons under `tests/`: - `smoke.nu` — `/health`, `/openrpc.json`, `/.well-known/heroservice.json`, `rpc.discover` over the UDS socket via `curl --unix-socket`. - `api_integration.nu` — first-rootobject CRUD cycle over HTTP. - `e2e_<flow>.nu` — multi-step user-journey skeleton. Each script accepts `--socket <path>` and falls back to `HERO_TEST_SOCKET` so the same file works under `lab --test` and under a hand-run `lab service --start --ephemeral`. Rename `MultiDomainBuilder::for_test()` → `for_ephemeral()` and `Test` marker → `Ephemeral` so the builder's name matches the lab flag every consumer reaches for. Regenerate `examples/recipe_server` so the example tracks the new templates. The example workspace is excluded from the top-level `cargo test --workspace`, so hero_rpc CI doesn't require lab to be installed — only `cargo check` runs on the example, which the subprocess shape compiles cleanly under.33e8d602c02b7096c5bcThe previous layout checked the scaffolded `examples/recipe_server/` tree directly into git — 60+ files, almost all of which are pure scaffolder output. Two problems: drift (the tree silently went stale whenever the scaffolder template changed) and signal-to-noise (the ~340 lines of hand-written content that actually teach the recipes domain got buried in scaffolder boilerplate). Replace that with a build script. The example is now the *act* of building a working recipes service: examples/recipe_example/ ├── build.sh ← runs scaffolder → injects hand-code → cargo build ├── README.md ← explains the pattern ├── schemas/recipes/recipes.oschema └── inject/ ← the ~340 lines a contributor would normally hand-write ├── crates/hero_recipes/src/recipes/types.rs (16 lines) ├── crates/hero_recipes_server/src/recipes/rpc.rs (180 lines — service methods) └── crates/hero_recipes_server/src/recipes/tests_error_category.rs (102 lines) `examples/recipe_server/` becomes the output of running `build.sh` — gitignored, regenerated from scratch on each invocation. Re-running guarantees the example tracks the current scaffolder; there is no stale-checked-in version to drift against. Two side notes: - The previous `examples/recipe_server/sdk/rust/tests/seed_smoke.rs` imported `hero_recipes_server::recipes::OsisRecipes` from inside the SDK crate, which the scaffolder-emitted Cargo.toml doesn't permit (no `_server` dev-dep). Dropped — the new workspace-root `tests/` crate (hero_rpc#124 subprocess-driver shape) covers the SDK ↔ server end-to-end path the smoke test was checking. - `build.sh` injects a `[patch."https://forge.ourworld.tf/lhumina_code/hero_rpc.git"]` section pointing at the in-tree `crates/*` so the example exercises the *working tree*, not whatever commit happens to be pinned on origin/development. Without it, the example always lags by a PR. - `cargo build` runs in three staged passes (`-p hero_recipes`, `-p hero_recipes_server`, `--workspace`) to side-step a reproducible cargo first-build race in workspace mode where the `hero_recipes_server` bin can't see its own sibling lib (same package name) under fresh `target/`. Staged building warms the per-package lib→bin sequencing.Pushed a follow-up commit + rebased onto current
development(picks up PR #127 / @index integration):b1afe11— recipe example is a build script, not a checked-in tree. Replaces 60+ checked-in scaffolder-output files underexamples/recipe_server/with a 3-step build script atexamples/recipe_example/build.sh: writes schemas → runs scaffolder → injects ~340 lines of hand-written content (service-method impls, custom type impls, a domain test). Output is gitignored. Eliminates the scaffolder ↔ checked-in-example drift exactly. Net file delta: 60+ files deleted, 8 added underexamples/recipe_example/.Rebased onto origin/development so PR #127s @index integration + osis_benches expansion are now in the base. The bench compile now passes locally (was the pre-existing breakage I called out in the original PR description — no longer a follow-up).
One small reframe on
run_for_test: it stays. The osis_benches harness is the server (no separate_serverbinary to subprocess-spawn), so the Phase F “migrate benches to subprocess” framing doesnt fit.run_for_testis the legit in-process variant oflab --ephemeralfor that case — same code path under the hood now that its a thin wrapper overMultiDomainBuilder::for_ephemeral().Summary of branch state: 4 commits (MultiDomainBuilder → subprocess scaffolder + nu skeletons → README/testing.md → example-as-build-script),
cargo test --workspace --lib --binsclean,cargo clippy --workspace -- -D warningsclean.Reframe + cleanup pass. `MultiDomainBuilder::for_ephemeral()` is the single internal-to-hero_rpc_osis primitive for spawning an OSIS service against a tempdir; `lab service <name> --start --ephemeral` is the only external entry point that consumers should reach for. The legacy single-domain `run_for_test<A>` wrapper is gone. Three changes: 1. `crates/osis/src/rpc/bootstrap.rs` — delete `run_for_test`. Drop the `bootstrap::run_for_test` re-export from `rpc/mod.rs`. The module docs reframe `MultiDomainBuilder` as the single bootstrap primitive shared between production `main.rs` and lab's ephemeral spawn path; there is no separate "in-process variant for tests". 2. `crates/osis_benches/benches/index_perf.rs` — drop the old `ServerFixture { _running: RunningServer, ... }` shape. The bench harness never actually called the server during timed sections (every measurement is a direct `Arc<OsisBench>` method call); the server was cosmetic. The bench now constructs `OsisBench` against a fresh tempdir via `OsisDomainInit::create` directly — no server in the loop, no tokio runtime needed in the bench groups. 3. Codegen-emitted `#![allow(clippy::clone_on_copy, clippy::needless_borrows_for_generic_args)]` at the generated barrel level (`generated/mod.rs` in both the types side via `generate/rust_types.rs` and the server side via `generate/rust_server.rs`) plus inside the `types.rs` wrapper that `include!()`s `generated/types.rs`. Codegen emits `.clone()` on Copy enum/primitive fields when building OTOML payloads — the redundancy is intentional so the emitter doesn't special-case Copy vs non-Copy. The allow keeps `cargo clippy -- -D warnings` green on every scaffolded service. Also touch up docs (BENCH_RESULTS.md, ADR-0002, generator/src/build/tests_emit.rs, osis Cargo.toml feature comment) so no stale `run_for_test` reference remains in the tree, and fix a pre-existing `doc_lazy_continuation` clippy error in `rust_struct.rs::generate_find_params` that surfaced once the generator lib started building cleanly under `--workspace`.Pushed
58dfe82—run_for_testis fully gone. Reframe:MultiDomainBuilder::for_ephemeral()is the only in-tree primitive, andlab service <name> --start --ephemeralis the only external entry point consumers should reach for.What changed:
bootstrap::run_for_test. Bench was the last caller. The bench was never actually using the server during timed sections (directArc<OsisBench>calls all the way down) — the server was cosmetic. Bench now constructsOsisBenchagainst a fresh tempdir viaOsisDomainInit::createdirectly. No server in the loop, no tokio runtime needed in the bench groups.#![allow(clippy::clone_on_copy, ...)]at the generated barrel + types-wrapper level socargo clippy -- -D warningsstays green after the bench unblocks (codegen emits .clone() on Copy types intentionally; the lint was masked previously by the pre-existing bench breakage).cargo test --workspace --lib --bins+cargo clippy --workspace -- -D warningsboth green.58dfe821ad5e72817184