relayburn-cli: HarnessAdapter trait + registry + pending-stamp factory (#248 part b)#307
Conversation
#248 part b) Wave 1 D4 of the Rust port (epic #240). Lands the harness substrate the Wave 2 fan-out PRs (#248-d/e/f for claude/codex/opencode) plug into. - `HarnessAdapter` trait (`crates/relayburn-cli/src/harnesses/mod.rs`) mirrors the TS shape from `packages/cli/src/harnesses/types.ts`: `name`, `session_root`, `plan`, `before_spawn`, `start_watcher` (optional), `after_exit`. Async methods desugar through `async-trait` so adapter impls stay readable. - Lazy compile-time `phf::Map` registry (`crates/relayburn-cli/src/harnesses/registry.rs`) with `lookup` / `list_harness_names` parallel to the TS registry. Slots are reserved but empty with comments naming the Wave 2 PR that populates each one. - `pending_stamp::adapter` factory (`crates/relayburn-cli/src/harnesses/pending_stamp.rs`) mirrors the TS `createPendingStampAdapter` shape: codex + opencode adapters will construct through it so the manifest writer + watch-loop wiring lives in one place. - `relayburn-sdk` re-exports the watch-loop + pending-stamp surface (`start_watch_loop`, `WatchController`, `write_pending_stamp`, `PendingStampHarness`, …) so the CLI doesn't have to reach into private SDK modules. - New `[lib]` target in `relayburn-cli` so the harness substrate can be unit-tested with `cargo test -p relayburn-cli` without disturbing `main.rs` (owned by the parallel #248-a CLI scaffold PR). - 9 unit tests cover the trait round-trip, the static-fake registry lookup, the pending-stamp factory for both codex + opencode, the `should_panic` on unknown harness names, and the empty-registry invariant on this branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces a harness adapter substrate for relayburn-cli, enabling pluggable adapters to manage spawned harness processes through a trait-based lifecycle (before_spawn, start_watcher, after_exit). It includes a phf-based registry for static adapter lookup, a concrete pending-stamp adapter, and expanded SDK re-exports for CLI integration. ChangesHarness Adapter Infrastructure
Sequence DiagramsequenceDiagram
participant CLI as CLI Driver
participant HA as HarnessAdapter Impl
participant Proc as Spawned Process
participant Watch as WatchController
participant Ingest as Ingest System
CLI->>HA: plan(ctx)
HA->>CLI: SpawnPlan
CLI->>HA: before_spawn()
HA->>HA: write_pending_stamp()
CLI->>Proc: spawn(plan)
Proc-->>CLI: pid
CLI->>HA: start_watcher()
HA->>Watch: start_watch_loop(options)
Watch->>Ingest: watch & ingest sessions
Ingest-->>Watch: IngestReport
Proc->>Proc: runs...
Proc-->>CLI: exit
CLI->>HA: after_exit(report)
HA->>Ingest: ingest_sessions_callback()
Ingest-->>HA: ✓
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…y (follow-up to #307) (#311) * relayburn-cli: make pending-stamp adapter compatible with phf registry The Wave 2 plan in `harnesses/registry.rs` calls for codex/opencode to register through `pending_stamp::adapter`, but that factory returned a runtime `Box<dyn HarnessAdapter>` while the `ADAPTERS` map is a compile-time `phf::Map<&'static str, &'static dyn HarnessAdapter>`. A `Box` cannot satisfy the `&'static` value bound, so the codex / opencode adapters would not have compiled when their PRs landed. Add `pending_stamp::adapter_static` that `Box::leak`s the impl and returns `&'static dyn HarnessAdapter`. Wave 2 adapters wrap the call in a `LazyLock<&'static dyn HarnessAdapter>` next to their module declaration. The leak is bounded (one Box per harness, ~3 ever) and adapters live until process exit anyway, so it's the right trade-off versus converting the registry to a runtime `OnceLock<HashMap<...>>`. Add a regression test in `harnesses/registry.rs` that proves a `pending_stamp::adapter_static` value satisfies the same value-type bound `ADAPTERS` requires. Follow-up to #307. Addresses the CodeRabbit review comment at #306 (comment). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * relayburn-cli: split eager vs runtime adapter registries (review fix) The previous registry doc-comments claimed codex/opencode would land in the `phf::Map`, but pending-stamp adapters are constructed at runtime via `Box::leak` and cannot appear in `phf_map!` (which needs const expressions). Split the registry into two tiers that match the real constraint: * `EAGER_ADAPTERS: phf::Map<…>` — for stateless unit-struct adapters whose value is a const reference (claude lands here). * `RUNTIME_ADAPTERS: LazyLock<HashMap<…>>` — for adapters built via `pending_stamp::adapter_static` whose `&'static dyn HarnessAdapter` is produced at runtime (codex / opencode land here). `lookup` checks the phf tier first, then falls back to the runtime tier; common-case lookups still pay only a single perfect-hash probe. Rewrites the regression test to exercise the actual production wiring path: `FAKE_RUNTIME_REGISTRY` uses the same `LazyLock<HashMap< &'static str, &'static dyn HarnessAdapter>>` value type that production uses, so insertion of a leaked `pending_stamp::adapter_ static(...)` reference proves compatibility with the registry it will actually live in. Adds a compile-time assertion (`_ASSERT_ADAPTER_STATIC_FITS_REGISTRY`) that pins `adapter_static`'s signature to the registry's value bound — if the factory regresses to `Box<dyn HarnessAdapter>`, both the runtime test fixture and the const assertion fail to compile. Addresses Codex review comment on PR #311. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * relayburn-cli: list_harness_names doesn't init runtime registry + deterministic ordering (review fix round 2) list_harness_names previously called RUNTIME_ADAPTERS.keys(), which forces LazyLock init via Deref and defeats the cold-start goal stated in the registry's own doc comment (burn --help shouldn't pay harness-table construction cost). It also leaked HashMap's HashDoS-randomized iteration order into user-facing help output, so the harness list could flicker between runs once Wave 2 adapters land. Fix: * Add a sibling RUNTIME_ADAPTER_NAMES: &[&str] static next to RUNTIME_ADAPTERS. Doc comment calls out the Wave 2 sync requirement: every codex (#248-e) / opencode (#248-f) PR must uncomment two rows in lockstep — its RUNTIME_ADAPTERS insert and its RUNTIME_ADAPTER_NAMES entry. * list_harness_names now extends with RUNTIME_ADAPTER_NAMES.iter(). RUNTIME_ADAPTERS is no longer touched on the help path. * Replace the empty-list test with list_harness_names_is_deterministic, which snapshots the expected ordering against an EXPECTED_HARNESS_NAMES constant and asserts two consecutive calls return the identical Vec. Doc comment leaves a cold-start proof: by inspection, list_harness_names doesn't touch the LazyLock, so the test passes without ever forcing init. * Add runtime_adapter_names_match_runtime_adapters as a sync-invariant test: every name in RUNTIME_ADAPTER_NAMES must resolve through lookup. Empty no-op today, pinned for Wave 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wave 1 D4 of the Rust port (epic #240, sub-issue #248). Lands the harness substrate that Wave 2's three adapter PRs (claude / codex / opencode) plug into.
Split
#248is split three ways for parallelism:commands/,cli.rs,main.rs,render/).HarnessAdaptertrait, lazy registry, pending-stamp factory.burn <command>output Wave 2 commands diff against.Once a + b + c land, Wave 2 can start the eight-PR fan-out. b unblocks the three adapter PRs (#248-d/e/f).
Summary
HarnessAdaptertrait atcrates/relayburn-cli/src/harnesses/mod.rsmirrorspackages/cli/src/harnesses/types.ts:name,session_root,plan,before_spawn,start_watcher(optional),after_exit. Async methods desugar viaasync-trait. Supporting types (PlanCtx,SpawnPlan,WatcherController) line up with the TS sibling.phf::Mapregistry atcrates/relayburn-cli/src/harnesses/registry.rswithlookup/list_harness_names. Slots are reserved but empty with comments naming the Wave 2 PR that populates each one (claude→ [Rust port] relayburn-cli: clap CLI + mcp-server subcommand (closes #210) #248-d,codex→ [Rust port] relayburn-cli: clap CLI + mcp-server subcommand (closes #210) #248-e,opencode→ [Rust port] relayburn-cli: clap CLI + mcp-server subcommand (closes #210) #248-f).pending_stamp::adapterfactory atcrates/relayburn-cli/src/harnesses/pending_stamp.rsmirrorscreatePendingStampAdapter. Codex + opencode ([Rust port] relayburn-cli: clap CLI + mcp-server subcommand (closes #210) #248-e/f) construct adapters through it so the manifest writer + watch-loop wiring lives in one place.before_spawncallsrelayburn_sdk::write_pending_stamp;start_watcherreturns a controller fromrelayburn_sdk::start_watch_loop.relayburn-sdkre-exports the watch-loop + pending-stamp surface (start_watch_loop,WatchController,write_pending_stamp,PendingStampHarness, …) so the CLI doesn't reach into private SDK modules.[lib]target inrelayburn-cliso the harness substrate can be unit-tested withcargo test -p relayburn-cliwithout disturbingmain.rs(owned by the parallel [Rust port] relayburn-cli: clap CLI + mcp-server subcommand (closes #210) #248-a scaffold PR).What Wave 2 adapter PRs need to know
HarnessAdapterdirectly (the eager-stamp path);start_watcherreturnsNone. Add a staticCLAUDE_ADAPTERand one row toADAPTERSinregistry.rs.PendingStampAdapterconfig and callpending_stamp::adapter(config)to get anHarnessAdapter. Box-leak it to a&'static; register inADAPTERS. Theingest_sessionsclosure should callrelayburn_sdk::ingestwith the harness's roots only.mod harnesses;intomain.rs; theburn rundriver lands in [Rust port] relayburn-cli: clap CLI + mcp-server subcommand (closes #210) #248-d (Wave 2 D5) and is where harness lookup gets hooked up.Test plan
cargo build --workspaceclean (no warnings)cargo test -p relayburn-clipasses (9 tests: trait round-trip, registry lookup with a static fake, codex/opencode factory round-trips,should_panicon unknown harness, empty-registry invariant,after_exitinvokes the supplied ingest fn)cargo test --workspacestill passes (619 tests total: 9 CLI + 605 SDK lib + 2 SDK integration + 3 doc tests)main.rs/cli.rs/commands/*(owned by the parallel [Rust port] relayburn-cli: clap CLI + mcp-server subcommand (closes #210) #248-a worktree)Closes part b of #248. Parent epic: #240.
🤖 Generated with Claude Code