relayburn-cli: opencode HarnessAdapter via pending-stamp factory (#248 D7)#320
relayburn-cli: opencode HarnessAdapter via pending-stamp factory (#248 D7)#320willwashburn merged 3 commits intomainfrom
Conversation
…D7) Wires the OpenCode harness adapter through the same `pending_stamp::adapter_static` factory codex uses (#248 D6). The new `harnesses/opencode.rs` mirrors `harnesses/codex.rs` exactly: * `name = "opencode"`, dispatch key + log label * `session_root` resolves to `$HOME/.local/share/opencode/storage/session`, matching the TS sibling at `packages/cli/src/harnesses/opencode.ts` * `ingest_sessions` opens a fresh `Ledger` handle per tick and runs `relayburn_sdk::ingest_opencode_sessions` (already re-exported) Registry plumbing: * `RUNTIME_ADAPTERS` gains `("opencode", opencode::adapter())` * `RUNTIME_ADAPTER_NAMES` gains `"opencode"` (alphabetical after codex) * `EXPECTED_HARNESS_NAMES` test fixture updated to `["codex", "opencode"]` Tests reuse the `ENV_LOCK` + `with_test_home` pattern introduced in the codex fixup (commit 5144ab6) so $HOME mutation is serialized across the parallel cargo test runner. Two new tests: * `config_has_opencode_name` — name + session_root resolution * `adapter_round_trip` — full trait-object round trip
|
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 selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new OpenCode harness adapter to relayburn-cli: new ChangesOpenCode Harness Adapter
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Registry
participant Adapter as OpenCodeAdapter
participant Ledger
participant SDK
participant FS as Filesystem
CLI->>Registry: lookup("opencode") / list_harness_names
Registry->>Adapter: return &'static HarnessAdapter
CLI->>Adapter: invoke ingest tick
Adapter->>Ledger: open fresh Ledger handle
Adapter->>SDK: relayburn_sdk::ingest_opencode_sessions(Ledger, options)
SDK->>FS: read session files under session_root ($HOME/.local/share/opencode/...)
SDK-->>Adapter: ingest results
Adapter-->>CLI: report status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/relayburn-cli/src/harnesses/opencode.rs`:
- Around line 92-109: The ENV_LOCK mutex and with_test_home function currently
only serialize within this module but HOME is process-global; extract the
Mutex<()> into a shared test utility (e.g., a pub static ENV_LOCK in a
test_utils or common tests module/crate using the same static initialization
method) and have with_test_home acquire that shared ENV_LOCK instead of the
local one so all tests that mutate HOME reuse the same lock; update
callers/imports to reference the shared ENV_LOCK and keep the rest of
with_test_home (prev capture, set_var, restore/remove_var, resume_unwind)
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d849d0a-5267-4498-937a-17b889bfb48c
📒 Files selected for processing (4)
CHANGELOG.mdcrates/relayburn-cli/src/harnesses/mod.rscrates/relayburn-cli/src/harnesses/opencode.rscrates/relayburn-cli/src/harnesses/registry.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0082b4d6be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// `relayburn_sdk::query_verbs::state_status` tests and the codex | ||
| /// sibling. Poisoned-mutex recovery is intentional — a panicking | ||
| /// test shouldn't break every subsequent run. | ||
| static ENV_LOCK: Mutex<()> = Mutex::new(()); |
There was a problem hiding this comment.
This ENV_LOCK only serializes tests inside opencode.rs; codex.rs defines a separate lock, so cargo test (which runs tests in parallel by default) can still run both modules’ with_test_home helpers concurrently and race on process-global HOME. In that case session_root() assertions can intermittently read the other module’s value, and on Unix set_var/remove_var during concurrent env access is explicitly unsafe. Please use a single shared lock (or serial test attribute) for all tests mutating HOME in this crate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 56cfc2a. The two per-module locks are gone; both codex::tests and opencode::tests now share a single crates/relayburn-cli/src/harnesses/test_env::ENV_LOCK. with_test_home lives in the same module (gated by #[cfg(test)]) and preserves the existing catch_unwind + resume_unwind shape so a panicking assertion still restores HOME before propagating.
Concretely: cargo test of the relayburn-cli binary now serializes every harness HOME mutation through one mutex — concurrent session_root() assertions across the two adapter modules can no longer race. Verified with cargo test --workspace (610 SDK + 23 CLI lib + 58 binary + 10 smoke + 5 golden tests; all green).
Resolve registry.rs conflicts by unioning all three Wave 2 adapters: - EAGER_ADAPTERS keeps claude (from #318 D5) - RUNTIME_ADAPTERS keeps codex (#317 D6) + opencode (this branch, D7) - RUNTIME_ADAPTER_NAMES = ["codex", "opencode"] (alphabetical) - EXPECTED_HARNESS_NAMES = ["claude", "codex", "opencode"] CHANGELOG.md additive: keep opencode bullet alongside D5/D6/D8 bullets. mod.rs auto-merged with all three pub mod statements. Cargo.lock taken from origin/main and confirmed unchanged after build.
CodeRabbit + Codex flagged: codex.rs and opencode.rs each defined a private `static ENV_LOCK: Mutex<()>` for serializing `HOME` mutation in tests, but `HOME` is process-global. `cargo test` runs modules in parallel by default, so the two modules' `with_test_home` helpers could acquire their own locks simultaneously and race on the global env, intermittently leaking each other's sentinel values into `session_root()` assertions. Extract the lock + helper into a shared `harnesses::test_env` module gated by `#[cfg(test)]`. Both adapter test modules now `use crate::harnesses::test_env::with_test_home;` and funnel through one crate-wide `ENV_LOCK`. The save-set-restore-`resume_unwind` machinery (including `catch_unwind` so a panicking assertion still restores `HOME` before propagating) lives in one place — adapters that join later (e.g. a future `aider` harness) reuse the helper without re-deriving the unwind-safety details. Scope kept tight: SDK-side `RELAYBURN_*` env tests in `relayburn_sdk::query_verbs` carry their own lock and don't share this one. Lifting both into a workspace-wide test-utility crate is a deliberate follow-up if HOME and RELAYBURN_HOME mutations ever need to interlock; today the two scopes are independent.
1 participant
Scope
Wave 2 D7 of the burn → Rust port (epic #240): the OpenCode harness adapter, registered via the
pending_stamp::adapter_staticfactory the codex adapter (#317 D6) already uses.Files
crates/relayburn-cli/src/harnesses/opencode.rs(new) — mirror ofharnesses/codex.rs.name = "opencode",session_rootresolves to$HOME/.local/share/opencode/storage/session(matches the TS sibling exactly),ingest_sessionsopens a freshLedgerhandle and runsrelayburn_sdk::ingest_opencode_sessions(already re-exported by D6).crates/relayburn-cli/src/harnesses/mod.rs— addpub mod opencode;.crates/relayburn-cli/src/harnesses/registry.rs:RUNTIME_ADAPTERSgains("opencode", opencode::adapter())RUNTIME_ADAPTER_NAMESgains"opencode"(alphabetical aftercodex)EXPECTED_HARNESS_NAMEStest fixture updated to["codex", "opencode"]CHANGELOG.md— one impact-first bullet under[Unreleased].How it plugs in
harnesses::lookup("opencode")resolves through theRUNTIME_ADAPTERSLazyLock(factory-built adapters can't be const-expressions forphf_map!).list_harness_names()returns["codex", "opencode"]fromRUNTIME_ADAPTER_NAMESwithout forcing theLazyLock, preserving the cold-start contract. The deterministic-order test pins both lists in lockstep.Test pattern
Both opencode tests use the same
ENV_LOCK: Mutex<()>+with_test_homehelper introduced in the codex fixup (commit5144ab6) so$HOMEmutation is serialized across cargo's parallel test runner. The closure-around-catch_unwindshape preserves prior$HOME(or removes it if it was unset) even when the inner test panics.Test plan
cargo build --workspacecleancargo test --workspacegreen (610 SDK + 15 CLI harness tests + 9 sdk-node + integration)harnesses::lookup("opencode")returnsSome(adapter)(newadapter_round_triptest)list_harness_names()returns["codex", "opencode"](list_harness_names_is_deterministicupdated)runtime_adapter_names_match_runtime_adapterspasses — both lists in lockstepconfig_has_opencode_name,adapter_round_trip) pass under parallel runner without flake (env-lock pattern)Out of scope
harnesses/claude.rs,commands/run.rs, orcli.rs::Run.commands/{ingest,mcp_server}.rs.