feat(plugins): add extensible client architecture - #1061
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughClient construction now uses a validated ChangesClient platform APIs
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant BotBuilder
participant ClientBuilder
participant Client
participant LifecycleRegistration
participant ClientLifecycle
BotBuilder->>ClientBuilder: configure dependencies and options
ClientBuilder->>Client: build and start services
Client->>LifecycleRegistration: begin_scope_if_current(generation)
LifecycleRegistration->>ClientLifecycle: on_ready(scope)
Client->>LifecycleRegistration: close_scope(generation)
LifecycleRegistration->>ClientLifecycle: on_closed(scope)
Client->>ClientLifecycle: shutdown()
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
|
| Filename | Overview |
|---|---|
| src/client/extension_lifecycle.rs | Adds generation scope management, ordered callbacks, lossless closure delivery, and terminal lifecycle coordination. |
| src/client/lifecycle.rs | Integrates generation cancellation and authoritative cleanup with connection and shutdown flows. |
| src/client/builder.rs | Introduces canonical client assembly, extension installation, activation, and rollback. |
| src/plugins/mod.rs | Defines plugin manifests, registration, capabilities, task ownership, resource tracking, and diagnostics. |
| src/plugins/events.rs | Adds bounded custom-event routing, overflow handling, sequencing, and queue statistics. |
| wacore/src/types/events.rs | Adds owned event subscriptions and aggregate event-interest tracking. |
Sequence Diagram
sequenceDiagram
participant Client
participant Lifecycle
participant PluginHost
participant Plugin
Client->>Lifecycle: Authenticate generation N
Lifecycle->>PluginHost: Ready(scope N)
PluginHost->>Plugin: on_ready(scope N)
Client->>Lifecycle: Reconnect or disconnect
Lifecycle->>PluginHost: Cancel scope N
PluginHost->>PluginHost: Drain generation tasks
PluginHost->>Plugin: on_closed(scope N)
Client->>Lifecycle: Terminal shutdown
Lifecycle->>PluginHost: Shutdown
PluginHost->>PluginHost: Drain install tasks
PluginHost->>Plugin: shutdown()
Reviews (35): Last reviewed commit: "refactor(plugins): leave capability expa..." | Re-trigger Greptile
📦 Binary size report
.text per crate
Top movers (cargo-bloat attribution)
Baseline: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de5d5bc5b7
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib.rs (1)
180-199: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider wiring the new builder/lifecycle types into
preludetoo.
preludestill only re-exportsClient/ClientError. If plugin hosts and foreign-language adapters are the target audience forClientBuilder/ClientLifecycle/ConnectionScope, they'll likely want these importable frompreludewithout reaching intocrate::clientdirectly. Not blocking — future phases may handle this deliberately — but worth a look before the plugin API surface locks in.♻️ Possible addition
pub mod prelude { pub use crate::bot::{Bot, BotBuilder, BotHandle, EventDelivery, MessageContext}; - pub use crate::client::{Client, ClientError}; + pub use crate::client::{ + Client, ClientBuilder, ClientBuilderError, ClientError, ClientLifecycle, ConnectionScope, + ConnectionScopeState, + };🤖 Prompt for 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. In `@src/lib.rs` around lines 180 - 199, Update the prelude module’s client re-exports to include the new ClientBuilder, ClientLifecycle, and ConnectionScope types alongside Client and ClientError, making them available through the standard prelude import path.
🤖 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 `@src/client/lifecycle.rs`:
- Around line 983-985: Update cleanup_connection_state’s lifecycle.close_scope
await so a stuck plugin cannot block reconnect indefinitely: bound the teardown
wait with the existing timeout mechanism or otherwise let reconnect progress
independently of plugin teardown. Preserve normal close_scope execution and
ensure the cleanup path continues after the bound is reached.
---
Outside diff comments:
In `@src/lib.rs`:
- Around line 180-199: Update the prelude module’s client re-exports to include
the new ClientBuilder, ClientLifecycle, and ConnectionScope types alongside
Client and ClientError, making them available through the standard prelude
import path.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4f76f103-7dbb-471e-9878-767f2302b137
📒 Files selected for processing (8)
src/bot.rssrc/client.rssrc/client/builder.rssrc/client/extension_lifecycle.rssrc/client/lifecycle.rssrc/client/node_io.rssrc/lib.rssrc/types/enc_handler.rs
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 `@src/client/extension_lifecycle.rs`:
- Around line 175-178: Extract a private scopes accessor on the enclosing type
that performs the existing self.scopes lock with poisoned-lock recovery, then
replace the duplicated lock expressions in begin_scope, cancel_active_scope,
close_scope, has_open_scopes, and scope_for with calls to that accessor.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 81faaf76-1958-4f71-9d0c-1d18efd298d9
📒 Files selected for processing (1)
src/client/extension_lifecycle.rs
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/extension_lifecycle.rs (1)
77-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNail down the public "scope ended" contract before we ship this seam.
is_cancelled()returns true once the state reaches Cancelled or Closed, and a cleanclose()(Ready→Closed) still firescancellation_signalbecauseprevious < SCOPE_CANCELLED. Internally that's fine and consistent. The problem is this is a public plugin surface — an extension author readingis_cancelled/cancellation_signalwill reasonably assume "was cancelled", not "the scope is over". That's the kind of wrong mental model that ships bugs in every plugin built on top of it. Spell the semantics out in the doc comment so people build against what it actually does.🤖 Prompt for 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. In `@src/client/extension_lifecycle.rs` around lines 77 - 111, Update the public documentation for is_cancelled and the cancellation signal to explicitly state that they indicate the scope has ended, including both cancellation and clean closure. Document that close() notifies the cancellation signal even when transitioning from Ready, so extension authors do not interpret these APIs as cancellation-only.
🤖 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 `@src/client/builder.rs`:
- Around line 353-356: Update the lifecycle initialization around
LifecycleRegistration::new to combine the existing Option::map operations into
one closure that constructs and wraps the registration in Arc directly,
preserving the current None behavior and resulting type.
---
Outside diff comments:
In `@src/client/extension_lifecycle.rs`:
- Around line 77-111: Update the public documentation for is_cancelled and the
cancellation signal to explicitly state that they indicate the scope has ended,
including both cancellation and clean closure. Document that close() notifies
the cancellation signal even when transitioning from Ready, so extension authors
do not interpret these APIs as cancellation-only.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3058862d-e026-4e71-8f26-bf4933377ba3
📒 Files selected for processing (5)
src/client/builder.rssrc/client/extension_lifecycle.rssrc/client/lifecycle.rssrc/client/node_io.rssrc/lib.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0da3e2e96
ℹ️ 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".
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bot.rs (1)
1284-1314: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the
alloc_meterandtask_instrumentassignments.Look, we're building the future of messaging here, not a labyrinth of nested matches. This assignment is way more complex than it needs to be. It also completely skips
task_instrumentifalloc_meteris present. If they aren't mutually exclusive, that's a bug. Either way, clean this up with sequentialif letblocks. We need things to work right and be readable.♻️ Proposed refactor
- client_builder = match alloc_meter { - Some(meter) => client_builder.with_alloc_meter(meter), - None => match task_instrument.clone() { - Some(instrument) => client_builder.with_task_instrument(instrument), - None => client_builder, - }, - }; + if let Some(meter) = alloc_meter { + client_builder = client_builder.with_alloc_meter(meter); + } else if let Some(instrument) = task_instrument.clone() { + client_builder = client_builder.with_task_instrument(instrument); + }🤖 Prompt for 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. In `@src/bot.rs` around lines 1284 - 1314, Replace the nested `alloc_meter`/`task_instrument` match assignment on `client_builder` with sequential `if let` blocks, applying `with_alloc_meter` when `alloc_meter` exists and independently applying `with_task_instrument` when `task_instrument` exists. Preserve both options when they are provided.
🤖 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.
Outside diff comments:
In `@src/bot.rs`:
- Around line 1284-1314: Replace the nested `alloc_meter`/`task_instrument`
match assignment on `client_builder` with sequential `if let` blocks, applying
`with_alloc_meter` when `alloc_meter` exists and independently applying
`with_task_instrument` when `task_instrument` exists. Preserve both options when
they are provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2b04c62a-ff1d-4c33-a17f-5b93ab3cf2c5
📒 Files selected for processing (24)
examples/voip-cli/src/main.rssrc/bot.rssrc/client.rssrc/client/accessors.rssrc/client/builder.rssrc/client/extension_lifecycle.rssrc/client/lifecycle.rssrc/client/node_io.rssrc/client/tests.rssrc/features/labels.rssrc/handlers/call.rssrc/handlers/notification/mod.rssrc/history_sync.rssrc/lib.rssrc/message/commit_batch.rssrc/message/durability.rssrc/message/tests.rssrc/pair_code.rssrc/passkey/flow.rssrc/receipt.rsstorages/chat-store/src/lib.rsstorages/chat-store/src/store.rstests/e2e/src/lib.rswacore/src/types/events.rs
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/extension_lifecycle.rs (1)
306-351: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
after_removeruns while thescopes(and thencallback_queue) mutex is held — document that contract.
close_scope_withkeeps thescopesMutexGuardalive across the caller-suppliedafter_remove()closure and the subsequentcallback_queue()lock acquisition. Todayclose_scopealways passes|| {}, so it's harmless, but the pattern is a live stall risk: any future caller that givesclose_scope_witha non-trivialafter_removewill block every other synchronous consumer ofscopes()(begin_scope_if_current,cancel_scope,ready(),enqueue_shutdown_if_ready) for the duration of that closure. Your own testfinal_scope_closure_is_published_before_shutdownhad to bump toworker_threads = 2specifically becauseshutdown()'scancel_active_scope()synchronously blocks on this held lock — on a single-threaded runtime (e.g. the WASM target mentioned in the PR objectives) this would stall the executor outright.A short doc comment on
close_scope_withspelling out "must be non-blocking; runs while holding the scope/callback-queue locks" would prevent this from becoming a real bug later.📝 Proposed doc comment
+ /// `after_remove` runs synchronously while holding `scopes` (and then + /// `callback_queue`) internally — it MUST be non-blocking. A slow or + /// blocking closure here stalls every other lifecycle operation + /// (`begin_scope_if_current`, `cancel_scope`, `ready`, `shutdown`) and, + /// on single-threaded runtimes, the executor itself. fn close_scope_with(self: &Arc<Self>, generation: u64, after_remove: impl FnOnce()) {🤖 Prompt for 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. In `@src/client/extension_lifecycle.rs` around lines 306 - 351, Document the locking contract on close_scope_with: after_remove must be non-blocking because it executes while the scopes mutex is held and before or during acquisition of the callback_queue lock. Place a concise doc comment directly above close_scope_with and state that callers must not perform blocking or reentrant operations that require these locks.
🤖 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.
Outside diff comments:
In `@src/client/extension_lifecycle.rs`:
- Around line 306-351: Document the locking contract on close_scope_with:
after_remove must be non-blocking because it executes while the scopes mutex is
held and before or during acquisition of the callback_queue lock. Place a
concise doc comment directly above close_scope_with and state that callers must
not perform blocking or reentrant operations that require these locks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a13e8a9d-c01e-4dfd-8e87-860538b85ae8
📒 Files selected for processing (2)
src/client/extension_lifecycle.rssrc/client/lifecycle.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1a8138b77
ℹ️ 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".
…ture # Conflicts: # src/client/device_registry.rs # src/client/lifecycle.rs
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60fda0bce4
ℹ️ 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".
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Status
Ready for review; Phase 3 and the selected pre-merge extensibility foundations are complete.
This remains a single, cumulative PR for the plugin-architecture refactor. Phase 4 items are demand-driven and outside this delivery. Current head
30d06ff6includesorigin/mainataa2582ca(including #1070 and #1069). The complete local suite, all-test Clippy, minimal native/WASM plugin builds, and all 40 plugin-host tests are green on current head30d06ff6; the final A/B remains green at60fda0bc. All review threads are resolved, and GitHub checks plus fresh AI reviews are rerunning on the published head. No merge intomainwas performed.PluginEventRouterkeyed by(plugin_id, topic)Readyadmission with lossless per-generationCloseddeliveryclient-lifecycleorpluginsis enabledagent_docs/plugin_architecture.mdThe bridge and baileyrs are not implementation dependencies of this PR. The native seams are being designed so a foreign-language adapter can be added later without rebuilding the client lifecycle or plugin model. The foreign command/protocol boundary remains deliberately deferred until a real bridge consumer is in scope.
Why
Client construction was split between high-level and low-level paths, and background services could only be coordinated through client-specific initialization. There was also no public lifecycle model aligned with the client's existing connection generation, which a plugin host needs to cancel connection-owned work reliably across expected disconnects, failures, reconnects, and terminal shutdown.
The existing event bus also had permanent registrations, evaluated handler interest dynamically under a read lock, and exposed raw-node forwarding as a global last-write-wins boolean. Those contracts could not safely support independently owned plugin or foreign-consumer subscriptions.
Phases 0 and 1 establish those foundations. Phase 2 now adds the native host and bounded custom-event routing without introducing the foreign protocol or bridge implementation yet, keeping that later boundary driven by a real foreign vertical slice.
What changed in Phase 0
Subscription, a#[must_use]RAII token that unregisters on dropEventInterestat subscription time and support controlled updates throughSubscription::update_interestAtomicU64words, makinghas_handler_forlock-free without requiring unavailableAtomicU128RawNodeLeaseownership, so independent consumers cannot disable one anotherEventInterestcapacity from 64 to 128 kindsWhat changed in Phase 1
ClientBuilderand typedClientBuilderErrorBotBuilderthrough the canonical pipeline while preserving its typestate APIClientLifecycleseam installed throughWeak<Client>, with rollback before publication on installation failureConnectionScopearound the existing connection generation: ready after authentication, synchronous cancellation immediately after generation bump, and close only after authoritative cleanupReadywork at 64 entries, and preserve every per-generationClosedcallback before terminalShutdownClosedpublication atomic with terminalShutdown, preserving lifecycle order under racesWhat changed in Phase 2.1
ClientPlugin { type Api; manifest(); install(); on_ready(); on_closed(); shutdown() }for trusted native plugins registered at build timeClientBuilderand the typestate-preservingBotBuilderclient.plugin::<P>() -> Option<Arc<P::Api>>, keyed by the plugin marker rather than the API type, so two plugins may expose the same Rust type without collisionClientLifecycleoutside the plugin order instead of replacing itWeak<Client>plus plugin resource state, avoidingClient -> plugin API -> Clientcycles and rejecting use before publication or after shutdownConnectionScopeon_closed, rollback, or LIFO shutdown hooksSend + Syncand the existing WASMMaybeSendSyncconvention, without unsafe codeplugins; default/no-plugin builds have no plugin field, branch, allocation, or linked codeWhat changed in Phase 2.2
PluginEventRouterseparate fromCoreEventBus, so custom plugin traffic neither changes the sealed coreEventcontract nor consumesEventInterestbitsPluginCapability::PluginEventsPluginEvents; a plugin cannot publish into another plugin namespace(plugin_id, topic)selectors before a native endpoint or future FFI adapter is awakenedDropNewest/DropOldestbehavior and cumulative delivery/drop/depth statisticsArc<PluginEventEnvelope>payloads across matching endpoints and carry schema version, encoding, connection generation, and route-local sequenceWhat changed in Phase 2.3
PluginFuture<'a, T>alias so public plugin futures areSendon native and remain valid on single-threaded WASMPluginTasks::sleepandPluginConnectionTasks::sleep; install-scoped clocks survive reconnects while connection-scoped clocks terminate with their generationplugins/metricsas an external, unpublished workspace crate that consumes only the public plugin API, proving the host is usable outside the main crateMetricsApithroughclient.plugin::<MetricsPlugin>()with sealed snapshot payloads and a reusable typed tick selectorReview hardening through Phase 3.2
ClientBuilder::build()closes resources immediately and schedules bounded LIFO shutdown for the current plugin, previously installed plugins, and the installed upstream lifecycleRawNodeLeasewhenever a plugin subscribes toEventKind::RawNodeClientBuild::client()path with consuminginto_client(), which starts the default major-sync worker; advanced hosts still useinto_parts()and own the sole receiverClientLifecycle::installas typedClientBuilderError::LifecycleInstallfailuresClientLifecycle::signal_shutdownboundary so FFI-style synchronous shutdown closes plugin tasks, subscriptions, leases, and capability handles even while other references remain aliveDuration::ZEROfor the background saver instead of starting a permanent hot loopdisconnect()await, closing plugin resources immediately while preserving the durability flush and connection-shutdown orderingConnectedpublication against generation and terminal cancellation, while allowing aConnectedhandler to signal shutdown reentrantly without deadlockPluginContextdependency views weak and non-owning, so an API may retain its context without creating anApiRegistry -> API -> context -> registrycycleArc<Client>drops, and make the device-registry waiter own only a shutdown signal so it cannot keep the client alive foreverClient::run()on successful construction/lifecycle activation, so a leaked client cannot connect while plugin installation is incomplete or after construction was rejectedclient-lifecycle;pluginsenables it transitively, while default clients retain the lifecycle-free field/layout and cleanup fast pathConnectionScopefrom an unwind guard immediately after the generation bump, so cancellation or a transport panic cannot strand terminal shutdownConnectionScopecancellation signal, so task cleanup does not depend on lifecycle callback schedulingReadywork remains bounded, while losslessCloseddelivery may temporarily exceed the target before orderedShutdownWhat changed in Phase 3.3
Client::plugin_stats()with per-manifest lifecycle state, sticky health, callback/task failures, active task generations, subscriptions, and isolated teardown/core-event panicsPluginEvents::stats()plus aggregatePluginEventRouter::stats()for delivery, backpressure, endpoint capacity, and queue occupancyMemoryReport, fully gated behindpluginsagent_docs/plugin_architecture.mdLatest review hardening
Connectedpublication to the authentication task generation, serialized against authoritative cleanup; a stale success task cannot claim a replacement scopeplugin_id, incrementPluginStats::task_panics, and make plugin health sticky-degradedClient::connect()on successful construction activation, matchingrun()and preventing leaked install-time handles from starting transport earlyDropOldestandDropNewestlosses to the publisher that owned the discarded envelope, including cross-namespace endpoint queues; the per-call report still records the eviction caused by that callclient-lifecycle, preserving the default client layout and fast pathClosedcallback under pressure while coalescing replaceable readiness work to the latestReady, even when closures already exceed the soft queue targetUntypedClientPluginregistration, includingArc<dyn UntypedClientPlugin>, so a future bridge can host multiple runtime-defined instances without nativeTypeIdcollisionsRawNodeLeaseupdates, and host shutdown invalidates retained tokensCargo feature policy
pluginsremains opt-in, andclient-lifecycleremains the smaller opt-in seam for hosts that do not need the plugin host. External plugin crates should enablewhatsapp-rust/pluginson their dependency; Cargo feature unification then activates it transitively, so application users do not need to discover a second switch. docs.rs builds withpluginsand labels gated APIs.LTO is an optimization, not an API or size guarantee. In the current release harness, enabling
pluginswithout installing a plugin had equivalent throughput but increased the stripped binary by 180,104 bytes (+0.89%). Keeping the host opt-in therefore avoids a permanent size tax while preserving an uncomplicated consumer path. No additional fine-grained plugin feature matrix is planned.Known limitations and pre-merge decisions
This checklist is intentionally honest about the current boundary. Checked items are foundations committed in this PR; unchecked items remain explicit limitations, not behavior implied by the API.
Foundations added before merge
TypeIdremains only the native typed API lookup key.Deliberately deferred until a real vertical slice
Native plugins are trusted in-process code. Capabilities shape the API they receive; they are not a security boundary. Runtime enforcement belongs at a future foreign-process/WASM boundary. The unchecked deferred items will not be silently approximated in this PR.
Consumer migration guide (breaking changes)
This guide compares the current
mainAPI with this PR's current head. It will be updated whenever a later commit changes a public contract. Phase 2.2/2.3 plugin additions are opt-in and do not add source-migration requirements for existing consumers;06050d0cadds the drop-time lifecycle guarantee and5b4a3b4fmakes scoped task teardown an awaited barrier before plugin hooks.47dcf10a,1ee952b9, andc86f6d52harden lifecycle ordering/construction without widening the public API.63bb4d95makes standalone lifecycle APIs explicitly opt-in throughclient-lifecycle; thepluginsfeature enables it transitively.54cbf95chardens unwind and terminal-install behavior without widening the public API.271d8423isolates destructor panics during resource teardown,e2f6bc4cmakes scope retirement and future cancellation unwind-safe, ande72f2729makes per-generation closure delivery lossless under queue pressure.667689abadds opt-in observability snapshots and counters,3244e32conly clarifies an internal lock contract, anddc55c1e3persists the architecture contract. None adds a source-migration requirement. The latest hardening adds the non-exhaustivePluginStats::task_panicsdiagnostic and clarifies cooperative task cancellation; ordinary consumers require no change, while plugin authors should make long-running work observe the provided shutdown signal or use the cancellation-aware task helpers.7f025d32gates direct connect during construction, andb4b9c63ccorrects cross-namespace overflow attribution. These are behavioral fixes and add no source-migration requirement; telemetry consumers should readPluginEventPublishReport::droppedas loss caused by that publish call andPluginEventPublisherStats::droppedas loss of envelopes owned by that publisher.f1a8138bdelays every public plugin surface until the final construction commit; normal post-build consumers require no change.29481d9eadds manifest-keyed untyped registration,03c4b8bbmakes plugin core-event subscriptions owner-scoped,1444153badds cooperative task draining,60fda0bcbounds installation/partial rollback,bfdb2699makes closed subscription registry cleanup immediate,21b430c2records that ownership contract in the RFC, and30d06ff6widens the private capability mask for future handles. These are additive relative tomain; plugin authors adopting this PR must retain subscription tokens and choose task cancellation semantics explicitly.Upgrade checklist
BotBuilder::with_event_handler(...)and closure-based bot handlersClient::register_handler(...)orCoreEventBus::add_handler(...)Subscription, or explicitly call.detach()for process-lifetime registration.EventHandler::interest()implementationSubscription::update_interest(...); interest is captured once at registration.Client::set_raw_node_forwarding(bool)RawNodeLeasefromacquire_raw_node_forwarding()for exactly as long as forwarding is needed.BotBuilderError::UnsupportedDurabilityBackendBotBuilderError::Client(ClientBuilderError::UnsupportedDurabilityBackend(_)).Client::new(...)/Client::new_with_cache_config(...)ClientLifecycle,ConnectionScope, orClientBuilder::with_lifecyclewithout pluginsclient-lifecycle. Consumers enablingpluginsalready receive it transitively.PluginEventSubscription, choose a bounded queue/overflow policy, and route by(plugin_id, topic, schema_version);EventInterest::ALLdoes not include custom events. Treat publish-report drops as call-attributed and publisher-stat drops as discarded-envelope-owner-attributed.PluginCoreEvents::subscribe(...)PluginCoreEventSubscriptionin plugin-owned state/API. Dropping it now unregisters immediately; useupdate_interestorunsubscribefor explicit changes.UntypedClientPluginand register by manifest ID withwith_untyped_plugin(...)/with_untyped_plugin_arc(...); this path intentionally exposes no RustClient::plugin::<P>()API.spawnfor abort-on-cancel work. Usespawn_cooperativeonly when the future observesshutdown_signal()/cancellation_signal()and can finish within the configured drain deadline.PluginHostConfig; install defaults to 30 seconds, callbacks/drains to 5 seconds, and zero durations are rejected. A timed-out install is rolled back.Client::disconnect()Arc<Client>withoutdisconnect().awaitClient::Dropnow emits the synchronous lifecycle signal, but durable flush/transport cleanup still require explicit async disconnect.1. Retain event subscriptions
Registrations now have an owner. Dropping the returned token unregisters the handler, so ignoring the result no longer creates a permanent registration.
Before:
After, with scoped ownership:
For an explicit filter:
If the old process-lifetime behavior is genuinely intended, make that choice explicit:
An in-flight dispatch may finish once from its previous immutable snapshot after unsubscribe; subsequent dispatches cannot see the removed handler.
2. Update dynamic event interest through the token
EventHandler::interest()is now a registration-time hint. The bus does not call it for every event. Code that previously mutated handler state and expectedinterest()to widen automatically must retain itsSubscriptionand update the filter explicitly:This contract enables the lock-free aggregate-interest fast path while keeping changes explicit and race-safe.
3. Replace the raw-node boolean with an ownership lease
Before:
After:
Each consumer owns an independent lease. One consumer can no longer disable forwarding while another still needs it. Plugin core-event subscriptions acquire the corresponding raw-node lease automatically.
4. Adjust
BotBuilderErrorpattern matchesDurability validation moved into the canonical client builder, so the old top-level variant is now nested.
Before:
After:
Both error enums are
#[non_exhaustive]; downstream matches should retain a fallback arm.5. Optional low-level builder migration
The existing
Client::new*constructors remain source-compatible. Dynamic hosts and FFI layers may instead use the new runtime-validated builder:Use
into_client()for a standalone client; it starts the standard major-sync worker. Only advanced hosts that take responsibility for draining the sole receiver should callinto_parts().6. Account for terminal lifecycle timing
Client::disconnect()now invokes the prompt, idempotentClientLifecycle::signal_shutdownboundary before its first cancellable await. Plugin tasks, subscriptions, raw-node leases, and capability handles therefore stop immediately when terminal disconnect begins; the durability flush, outbound flush, and connection-shutdown notification retain their existing order afterward. Consumers must not depend on plugin capabilities remaining usable during that terminal flush window.Dropping the final
Arc<Client>invokes the same synchronous signal so standalone lifecycle work cannot remain parked. This is not a substitute fordisconnect().await:Dropcannot perform the async durability flush or transport teardown.7. Enable standalone lifecycle APIs explicitly
Lifecycle extension APIs no longer contribute fields, branches, callback-driver code, or binary-size cost to the default client. Consumers that use lifecycle integration directly must opt in:
The
pluginsfeature includesclient-lifecycle, so native plugin consumers do not need to list both. Default consumers that use neither API require no source or manifest change.New plugin contract (opt-in, not a migration requirement)
Enable Cargo feature
plugins, register native plugins only during build, and retrieve their typed API by marker:Plugin API handles survive reconnects; connection-scoped work does not. Native plugins are trusted in-process Rust code, and capability handles shape available APIs without claiming sandbox enforcement.
Dependency lookup through
PluginContext::plugin::<P>()is deliberately non-owning at the context level. If an API retains itsPluginContextand needs a dependency later, it must retain the returnedArc<P::Api>during installation; retaining the context alone does not keep dependencies or the staging registry alive.The reference metrics plugin demonstrates the complete type-safe path:
Subscribe to custom plugin events
Custom events use their own exact, bounded router. They are intentionally not delivered through
CoreEventBus, andEventInterest::ALLcontinues to mean all core events only.Retain
PluginEventSubscriptionfor as long as the endpoint should exist. Select a queue capacity and overflow policy deliberately, inspectevents.stats()for loss/backpressure, and treatsequencegaps as discarded events. This is a new opt-in API; existing core-event consumers require no migration.Main synchronization
Merge commit
d4abef10brings in currentorigin/mainataa2582ca. That mainline includes #1070's removal of the obsolete device-registry cleanup task and #1069's typed stanza-response/retry work. The synchronization was conflict-free. The plugin host continues to expose only high-level messaging/IQ capabilities, and no new plugin branch was added to stanza or retry hot paths.Current head
30d06ff6is based on that main commit. The definitive non-E2E workspace suite, all-test Clippy, formatting, minimal plugin build, and plugin-enabled WASM build pass locally. The local E2E binary still requires the repository's Bartender service; the GitHub E2E job is authoritative and is rerunning on this push. No benchmark or project manifest was edited for this synchronization or the final A/B.Performance
Current whole-branch A/B after main synchronization
The definitive comparison uses clean
mainaa2582caand head60fda0bc. Both release clients were prebuilt from isolated source paths, passed throughCLIENT_BIN_OVERRIDE, and exercised on ports 18082/18083 without editing any benchmarkCargo.toml. Plugins were disabled, so this measures the ordinary consumer hot path. Five clean runs per side sent 30,000 ping-pongs at a 3,000/s cap; one additional PR run was discarded in full because host CPU pressure reached 5.17% versus roughly 0.16-0.22% in the retained batch.Main → PR medians were 2,859.33/s → 2,859.23/s throughput, 3.770 s → 3.580 s client CPU, 36.4 MB → 35.4 MB HWM, 0.54 ms → 0.56 ms ack latency, and 0.58 ms → 0.59 ms pong latency. The throughput ranges overlap (2,848.38-2,859.62/s versus 2,847.89-2,859.51/s), and all retained runs delivered 150,000 acknowledgements and pongs per side with zero loss or failures. The non-LTO release harness artifact changed from 27,094,008 to 26,807,696 bytes (-286,312 / -1.06%). No measurable feature-disabled regression is present.
Phase 3.3 observability
The isolated A/B compares
e72f2729(before native observability) with667689ab(observability). Both release clients enabledwhatsapp-rust/pluginsbut installed no plugin, so this measures the unused opt-in runtime path. Cargopathsoverrides, dedicated target/lock paths, and distinct ports kept the source and runs isolated; the pre-existing benchmarkCargo.tomlandCargo.lockretained SHA-256aea5f2e39dd88d569fcca4db37728c10ce04ea283033ee6858a77791ccaf2d1dandd81fa43e9802b569845eba1f24b219430d378e108cb87cf07cca1ae4ae52b19bthroughout.Ten order-alternated normal pairs ran on ports 18621–18640, with 30,000 ping-pongs at 3,000/s per run. Baseline → candidate side medians were 2,859.175/s → 2,859.280/s throughput, 3.460 s → 3.570 s client CPU, 26.65 → 26.75 MB RSS average, 35.45 → 35.45 MB HWM, and 9.95 → 9.95 MB anonymous RSS average. Four order-alternated stress pairs ran on ports 18641–18648, with 100,000 ping-pongs at 6,000/s per run: 5,846.965/s → 5,847.030/s, 9.000 s → 9.105 s client CPU, 39.85 → 39.80 MB RSS average, 55.20 → 55.05 MB HWM, and 22.95 → 23.00 MB anonymous RSS average. All 28 runs completed; each side delivered 700,000 acknowledgements and pongs with zero loss or pong failures.
The CPU samples do not establish a stable regression: normal paired deltas ranged from -0.12 s to +0.44 s with a +0.035 s median, while stress ranged from -0.39 s to +0.49 s with a +0.105 s median; median host interference also moved in opposite directions between the batches (4.0% → 3.8% normal, 4.65% → 5.3% stress). Throughput and memory were unchanged at the experiment's resolution.
For artifact size, both commits were additionally built from the same physical source path. With
pluginsenabled, observability adds 113,000 B to the unstripped ELF (+0.418%), 78,368 B of.text(+0.392%), 880 B of data, and 528 B of BSS. This is the explicit opt-in diagnostics cost. Withpluginsdisabled, allocated section sizes are identical on both sides (text=19,829,371,data=373,592,bss=3,944) and both stripped files are 20,208,840 B. The full ELFs are not byte-identical because Rust/LLVM codegen identity and layout metadata changed, so the supported claim is zero feature-disabled allocated-size growth rather than binary identity.Phase 3.2 lifecycle opt-in and binary-size gate
The same release/LTO demo used by the Binary Size workflow was rebuilt from a clean target at updated
mainand current head63bb4d95. Baseline → candidate measured 8,066,742 → 8,073,590 bytes of.text(+6,848 B / +6.69 KiB) and 10,089,576 → 10,097,032 stripped bytes (+7,456 B / +7.28 KiB). Before the standalone lifecycle split, the candidate measured 8,105,526 bytes of.text; feature-gating recovered 31,936 B and leaves roughly 25 KiB below the 32 KiB CI budget.The default client now contains neither the lifecycle field nor its cleanup/startup branches.
client-lifecyclerestores the standalone API andpluginsenables it transitively.The isolated runtime A/B compares
c86f6d52(before the split) with63bb4d95using Cargopathsoverrides, prebuilt binaries, separate targets, and five order-alternated pairs on ports 18501–18510. Across five 30,000-ping-pong runs per side, median throughput was 2,859.28/s → 2,859.43/s (+0.005%), client CPU 3.86 s → 3.92 s, RSS average 26.7 → 26.6 MB, HWM 35.5 → 35.0 MB, and anonymous RSS average 10.2 → 10.1 MB. Median host CPU interference was 7.4% → 9.4%, so the 0.06 s client CPU difference is within sampling/interference noise. Each side delivered 150,000 acknowledgements and pongs with zero loss.The non-LTO harness artifact shrank 87,200 B (-0.325%) and
.textshrank 78,380 B (-0.394%). The pre-existing benchmarkCargo.tomlandCargo.lockretained their exact SHA-256 hashes before and after the build/run sequence.Whole branch after updated-main synchronization
The benchmarked
9e7aba15head was built against the same dirty-but-unchangedwhatsapp-benchscheckout asmain87fbcb55, using Cargo path overrides, prebuilt binaries, and ports 18101–18106; neither benchmark manifest changed. Across three paired 30,000-ping-pong runs at 3,000/s, median actual throughput was 2,859.09/s → 2,859.27/s (+0.006%), median client CPU was 3.93 s → 3.86 s, median RSS average was 27.0 → 26.9 MB, and median HWM was 35.5 → 36.0 MB. Every run delivered all 30,000 acknowledgements and pongs with zero loss. The non-LTO release artifact shrank 194,328 bytes (-0.720%) and.textshrank 168,732 bytes (-0.843%). This shows no measurable hot-path regression after combining the plugin foundations with#1062; the HWM delta is within the observed run-to-run spread. The later2137fcb5hardening changes cold authentication/teardown synchronization, handler/subscription destruction, and install-time dependency ownership. Phase 2.2 at its isolated head9cef55dbis benchmarked independently below.Phase 2.3
The isolated A/B compares
9cef55dbwith57ec05dc. Both prebuilt release clients used Cargo source-path overrides and--features whatsapp-rust/pluginswithout installing a plugin; the external metrics crate therefore proves compile-time/public-API integration without entering the unused runtime path. Ten order-alternated pairs ran on dedicated ports 18401–18420 after AC power was restored. An earlier battery/power-saving batch was discarded in full and is not used here. The pre-existingwhatsapp-benchsmanifests retained their exact hashes before and after every batch.Across the ten valid 30,000-ping-pong runs per side, median throughput was 2,859.02/s → 2,859.31/s (+0.01%), client CPU 3.590 s → 3.585 s (-0.14%), RSS average 26.6 → 26.5 MB (-0.38%), HWM 35.25 → 35.05 MB (-0.57%), and anonymous RSS average 10.05 → 10.00 MB (-0.50%). Each side delivered 300,000 acknowledgements and 300,000 pongs with zero loss. Median host CPU interference was 4.45% → 4.50%; the deliberately retained first warm-up pair had higher interference, so medians rather than selected samples are reported.
The release harness artifact changed by +544 bytes (+0.002%);
.textchanged by +232 bytes, data by +40 bytes, BSS by -232 bytes, and total allocated sections by +40 bytes. These results show no measurable unused-runtime or memory regression from the task clock/API slice.Phase 2.2
The isolated A/B compares the pre-router head
2137fcb5with9cef55db. Both binaries used Cargo source-path overrides, temporary targets and lockfiles, and dedicated ports 18321–18336; the pre-existingwhatsapp-benchsCargo.tomlandCargo.lockhashes remained unchanged.With
pluginsdisabled, five paired 30,000-ping-pong runs produced median throughput of 2,859.20/s → 2,859.21/s. Three samples showed unrelated host CPU or memory pressure, so resource medians use the clean subset (four baseline and three candidate runs): CPU 3.54 s → 3.64 s, RSS average 26.35 → 26.5 MB, HWM 35.2 → 35.3 MB, and anonymous RSS average 9.9 → 10.1 MB. The non-LTO artifact changed by +9,896 bytes (+0.037%) and.textby +4,988 bytes (+0.025%), despite the feature-disabled code being absent; this is within link/code-layout noise.The stronger opt-in check rebuilt both sides with Cargo flag
--features whatsapp-rust/pluginsbut installed no plugin. Across three clean paired runs, median throughput was 2,859.29/s → 2,859.33/s, client CPU 3.56 s → 3.37 s, RSS average 26.4 → 26.4 MB, HWM 35.4 → 35.0 MB, and anonymous RSS average 10.0 → 10.1 MB. The candidate artifact was 15,824 bytes smaller (-0.059%) and.text4,280 bytes smaller (-0.022%). Every one of the 16 runs delivered all 30,000 acknowledgements and pongs with zero loss. The CPU differences are sampling/run-order noise; neither build mode shows a throughput, memory-retention, or unused-runtime regression.Phase 2.1
The Binary Size check first detected that compiling the native host into default builds added 83.16 KiB stripped and 71.56 KiB of
.text. That evidence triggered the intended opt-in feature boundary instead of a budget waiver. A same-toolchain release/LTO demo build at the pre-host head66f464a9and the feature-gated head7c2e584cis now byte-for-byte identical with plugins disabled: 10,112,136 stripped bytes and 8,087,734.textbytes on both sides. The normalwhatsapp-benchsA/B used prebuilt source overrides and separate ports; itsCargo.tomlremained untouched. Across three paired runs (30,000 ping-pongs at 3,000/s per side), median actual throughput was 2,859.23/s → 2,859.42/s (+0.01%), sampled client CPU was 3.85 s → 3.70 s, and median HWM was 35.6 → 35.1 MB. Every run delivered all acknowledgements and pongs with zero loss. The non-LTO harness artifact changed by +17,096 bytes (+0.063%);.textchanged by +2,368 bytes, consistent with codegen/link-layout noise from the declared opt-in feature rather than a default runtime path.Phase 1
Phase 1 A/B measurements used isolated copies of
whatsapp-benchsagainst the updatedmainatcd29cef4, with three normal runs per side:Phase 0 and lifecycle review hardening
The focused A/B compares the pre-Phase-0 head
a0da3e2ewith3bd4bd58. Both sides used the samewhatsapp-benchscheckout and prebuilt source overrides; the benchmarkCargo.tomlwas not changed. Normal measurements are medians from three paired runs on separate ports:The normal-run CPU differences are at sampler resolution and throughput is unchanged. DHAT and equal live-at-end show no lifecycle retention regression.
Validation
Current-head validation additionally passed:
cargo fmt --all -- --checkandgit diff --checkcargo clippy --all --tests -- -D warningscargo test --workspace --exclude e2e-tests(including 1,500 passed / 2 ignored inwacoreand 1,269 passed / 1 ignored inwhatsapp-rust)whatsapp-rust-plugin-metricscratecargo check -p whatsapp-rust --no-default-features --features pluginscargo check -p whatsapp-rust --lib --target wasm32-unknown-unknown --no-default-features --features pluginswithgetrandom_backend=wasm_jsFocused regressions now also cover manifest-keyed trait-object adapters, owned plugin subscription updates/unsubscribe, immediate registry cleanup after burst churn, capability-bit composition, cooperative connection/install task drains, bounded drain degradation, zero-deadline validation, installation timeout cleanup, and upstream partial-install rollback.
Dropmay unsubscribe from the same bus or reenter plugin subscription APIs without deadlock, including the post-shutdown rejection pathConnected, and remains safe when shutdown is signalled from the event handler itselfon_closed, install tasks finish before terminal hooks, rollback waits for task destruction, and task-drain timeout remains boundedReadyadmission remains capped, ordinary close ordering is retained, and all 254 queued generation closures are delivered before terminalshutdownreconnect()andreconnect_immediately()invoked fromon_readysynchronously cancel the active generation and suppress its staleConnectedpublicationon_closed, so bounded lifecycle queue eviction cannot retain stale generationsPluginContextis released with the client instead of cycling through the staging registryis_logged_insetcargo test --allbuilt and linked the complete workspace locally; only the E2E runtime failed because no Bartender mock server was reachable. Pulling the repository-pinned private image remains HTTP 403 with the available GitHub token, so the GitHub E2E job is authoritative.1ee952b9; all checks were green there, including WASM, formatting, clippy, all-features, no-SIMD, binary size, and CodSpeed30d06ff6, which containsorigin/mainataa2582ca. All review threads are resolved. GitHub checks and AI reviews are rerunning on this head and remain under monitoring.