Conversation
Package QA only ran plain client mode, and its test explicitly forbade client --auto, so the automatic runtime-selection path upstream tests had no downstream coverage at all. Add MESH_LLM_SMOKE_MODE to the shared readiness smoke. Auto mode runs --auto --disable-iroh-relays --nostr-relay ws://127.0.0.1:1/. That relay parses, so the client registers it without dialing, but it cannot connect, so discovery fails closed after its own bounded timeout and auto-selection reaches its local-mesh fallback. Readiness is the same structured passive_mode/status=ready/role=client event the direct client path already asserts. Nothing touches the public mesh. native-package-qa.sh now runs the direct client smoke and then the auto smoke, so every row proves both paths without waiting on a live mesh. Auto mode gets a 90-second default readiness budget rather than 45 to cover the discovery timeout. MESH_LLM_SMOKE_AUTO_RELAY is the seam for a local relay fixture that would extend this into real join coverage. Asserting a genuine public-mesh join stays upstream's ci-client-auto-test.sh, which is not something package rows should depend on.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe readiness smoke now supports client and automatic runtime-selection modes. Package QA runs both modes. Auto mode uses bounded offline discovery through a pinned loopback relay, and tests validate its arguments, readiness, shutdown, and error handling. ChangesRuntime readiness coverage
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant NativePackageQA
participant ReadinessSmoke
participant MeshLLM
participant LoopbackRelay
NativePackageQA->>ReadinessSmoke: run with MESH_LLM_SMOKE_MODE=auto
ReadinessSmoke->>MeshLLM: launch --auto --disable-iroh-relays
MeshLLM->>LoopbackRelay: attempt pinned relay connection
LoopbackRelay-->>MeshLLM: connection unavailable
MeshLLM-->>ReadinessSmoke: emit status=ready role=client
Merge Risk: ⚪ Minimal · up to No actionable current-head risk remains from this change. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
2 participants
Why
Package QA only ran plain
client, andtests/client-readiness-smoke.test.tsexplicitly assertedassert.doesNotMatch(source, /client --auto/). So the automatic runtime-selection path had no downstream coverage at all, even though upstream tests it inci-client-auto-test.sh.That upstream script can't be copied here. It is deliberately a public-mesh join test: it waits up to 300s for Nostr discovery and rejects the standalone fallback as evidence, on the grounds that only a real peer proves discovery works end to end. Correct for upstream, wrong as a gate on every package row.
What changed
MESH_LLM_SMOKE_MODEon the shared smoke,client(default) orauto. Auto mode adds:The relay URL parses, so the client registers it without dialing (which avoids the "zero relays registered" hard error), but it cannot connect. Discovery fails closed after its own bounded timeout, auto-selection takes its local-mesh fallback, and the client API comes up emitting the same structured
passive_mode/status=ready/role=clientevent the direct path already asserts. Nothing reaches the public mesh.native-package-qa.shruns the direct client smoke and then the auto smoke, so every row proves both paths and no row waits on a live mesh. Auto mode defaults to a 90s readiness budget instead of 45 to cover the discovery timeout.MESH_LLM_SMOKE_AUTO_RELAYis the seam for a local relay fixture that would extend this into real join coverage later.QA
Run end to end against a real
mesh-llm0.76.1 binary, not just fixtures:The 5s difference is the bounded Nostr fetch timeout. I confirmed the log shows
Connection failed url=ws://127.0.0.1:1/and no attempt against any public relay, thenNo meshes found yet — starting client API while discovery continues, then the readiness event.Unit coverage: auto mode passes the auto-selection and offline-relay flags (the fixture exits 64 if the relay is not loopback), an unknown mode is rejected with exit 2, and package QA runs both modes. The old
doesNotMatch(/client --auto/)guard is replaced by assertions that the default stays the direct client and that auto stays pinned to loopback, which is the property that actually mattered.Full suite 162/165; the 2
workflow-provenancefailures reproduce on unmodifiedorigin/mainon macOS and were green on ubuntu CI in #30 and #31.Caveat
One thing worth a second opinion: this proves auto-selection starts, not that it joins. Upstream's argument is that the standalone fallback is exactly the failure mode a weak predicate hides. That is a real gap, and closing it honestly needs the local relay fixture rather than a looser assertion.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation