fix(core): retry domain subscriber registration after bus startup

[samrusani]

Summary

• Keep a domain's subscriber-registration token unconsumed while the global event bus is unavailable.
• Preserve existing once-per-process registration after the bus is initialized.
• Cover deferred registration, successful retry, production global-bus wiring, and post-success idempotency.

Problem

group_first_time inserted a DomainGroup into the process-wide completed set before its registration block ran. If the block ran before the global event bus was initialized, every subscribe_global call returned None, but the group stayed marked complete and could never retry during the process lifetime.

Solution

Gate insertion into the completed set on global event-bus readiness. subscribe_global has only one failure condition: the monotonic global bus OnceLock has not been initialized. Once the bus exists it cannot disappear, so the existing registration blocks can run unchanged and remain idempotent.

The state transition is extracted behind an injectable readiness flag for deterministic failure-path coverage. A separate wrapper test exercises the real global-bus lookup with a domain token that has no subscriber block.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage >= 80% - changed lines reached 100% with the repository's scoped Rust coverage lane and diff-cover.
• Coverage matrix updated - N/A: behavior-only correction to existing core bootstrap registration.
• All affected feature IDs from the matrix are listed in the PR description under ## Related - N/A: no matrix feature ID applies.
• No new external network dependencies introduced.
• Manual smoke checklist updated - N/A: no release-cut or user-facing workflow changed.
• Linked issue closed via Closes #NNN in the ## Related section.

Impact

Core runtime only. A premature registration attempt now defers cleanly and can succeed on a later bootstrap call. Normal startup, domain widening, and once-per-process behavior are unchanged. No persistence, migration, API, or user-interface impact.

Related

• Closes core/jsonrpc: make DomainGroup subscriber registration retryable #5269
• Follow-up PR(s)/TODOs: None.

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: agent/retry-domain-subscriber-registration
• Commit SHA: 3e008bc24f283db5187ed8cab385f8f76f2403e2

Validation Run

• pnpm --filter openhuman-app format:check - N/A: no frontend files changed.
• pnpm typecheck - N/A: no TypeScript files changed.
• Focused tests: cargo test --lib domain_subscriber -- --nocapture (6 passed); repository scoped coverage lane (97 passed, 1 pre-existing ignored); diff-cover (100%).
• Rust fmt/check (if changed): cargo fmt --manifest-path Cargo.toml --check; git diff --check; default and slim-feature focused test builds passed.
• Tauri fmt/check (if changed): N/A: no Tauri files changed.

Validation Blocked

• command: N/A
• error: N/A
• impact: N/A

Behavior Changes

• Intended behavior change: bus-unavailable domain registration attempts remain retryable.
• User-visible effect: none during normal startup; prevents silently missing domain subscribers after premature registration.

Parity Contract

• Legacy behavior preserved: domain selection, later DomainSet widening, registration order, and once-per-process idempotency after successful bus initialization.
• Guard/fallback/dispatch parity checks: default-feature and --no-default-features --features tokenjuice-treesitter registrar tests both pass.

Duplicate / Superseded PR Handling

• Duplicate PR(s): None found in the final live search.
• Canonical PR: This PR.
• Resolution (closed/superseded/updated): N/A

Summary by CodeRabbit

• Bug Fixes
  • Improved domain event subscriber registration reliability when the event bus is temporarily unavailable.
  • Registrations now remain retryable until the event bus is ready, preventing missed subscriptions.
  • Successful registrations are safely handled as idempotent, avoiding duplicate setup.
  • Added coverage for deferred retries, repeated registrations, and event-bus readiness behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Domain and learning subscriber registration now checks global event-bus readiness before consuming completion tokens. Unavailable-bus attempts remain retryable. Successful registration remains idempotent. Tests cover both flows and the domain wrapper.

Changes

Domain subscriber registration

Layer / File(s)	Summary
Bus-aware registration tracking src/core/jsonrpc.rs	Registration helpers consume per-process completion tokens only when the global event bus is available. The domain wrapper uses this retryable flow.
Registration validation src/core/jsonrpc_tests.rs	Tests verify deferred retries, domain and learning idempotency, token preservation, and global event-bus usage.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Registration as register_domain_subscribers
  participant Helpers as registration helpers
  participant Bus as GlobalEventBus
  participant Completion as Per-process completion tracking

  Registration->>Helpers: Attempt subscriber registration
  Helpers->>Bus: Check event-bus readiness
  alt Bus unavailable
    Helpers-->>Registration: Defer without consuming token
  else Bus available
    Helpers->>Completion: Record group completion
    Helpers->>Bus: Register subscribers
  end

Loading

Possibly related PRs

• tinyhumansai/openhuman#4808: Both changes update domain-subscriber registration in src/core/jsonrpc.rs.

Suggested labels: rust-core

Suggested reviewers: senamakel

Poem

A rabbit checked the event bus light,
And saved the token when it was not right.
When ready, subscribers joined the stream,
Repeated calls stayed clean and lean.
Retry kept the promise bright.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the fix for retrying domain subscriber registration after event-bus startup.
Linked Issues check	✅ Passed	The changes defer completion until the event bus is ready, preserve retryability, and retain idempotency as required by issue #5269.
Out of Scope Changes check	✅ Passed	The changes remain within core subscriber registration behavior and add focused tests without unrelated system changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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/core/jsonrpc.rs`:
- Around line 1944-1972: Split src/core/jsonrpc.rs into focused Rust modules so
it is at most 500 lines, moving the bus-aware registration tracking around
group_first_time_when_bus_ready and group_first_time together with the
domain-subscriber registration logic into an appropriate module. Update imports,
visibility, and call sites to preserve existing retry behavior when the event
bus is unavailable and registration remains one-time after initialization.

🪄 Autofix

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: cac5fb14-6b11-4a9b-9975-88abd5f27a86

📥 Commits

Reviewing files that changed from the base of the PR and between d75b0a4 and 3e008bc.

📒 Files selected for processing (2)

• src/core/jsonrpc.rs
• src/core/jsonrpc_tests.rs

[greptile-apps]

Greptile Summary

This PR fixes a latent bootstrap bug where domain subscriber groups could be permanently skipped if register_domain_subscribers was called before the global event bus was initialized. The fix defers inserting a group into the DONE set until bus_ready is confirmed, making pre-bus calls fully retryable.

• group_first_time and learning_first_time are refactored into outer wrappers that delegate to injectable _when_bus_ready variants, enabling deterministic unit tests without touching real process-global state.
• Five new tests cover deferred registration, successful retry, idempotency after success, and the production global-bus wiring path.
• The fix also applies the same bus-readiness gate to learning_first_time, which previously had the identical silent-skip problem described in the PR's "Known limitation" comment (now removed).

Confidence Score: 5/5

Safe to merge. The change is a targeted, well-tested correction to a bootstrap ordering hazard with no persistence, API, or UI impact.

The fix is narrowly scoped to the completion-set insertion logic in two functions. The new _when_bus_ready helpers are pure and fully covered by the five added tests. The monotonic OnceLock guarantee that underpins the correctness argument is sound: once the bus is initialized it cannot disappear, so a true return from group_first_time_when_bus_ready always corresponds to a usable bus. Normal startup order is unchanged and idempotency after success is explicitly tested.

Files Needing Attention: No files require special attention. The INFRA Once block inside register_domain_subscribers still calls subscribe_global without bus-readiness gating, but that block is not retryable by design and its failure branch already logs a warning — this is pre-existing and out of scope for this PR.

Important Files Changed

Filename	Overview
src/core/jsonrpc.rs	Refactors group_first_time and learning_first_time to gate completion-set insertion on bus readiness; removes the now-resolved known-limitation comment; logic is correct and well-documented.
src/core/jsonrpc_tests.rs	Adds five focused unit tests covering deferred, retry, idempotency, and global-bus wiring paths; tests are clear and directly map to the new behaviour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[register_domain_subscribers called] --> B{bus_ready?}
    B -- No --> C[log warn: deferred registration\nreturn false]
    C --> D[Group NOT inserted into DONE\nRemains retryable]
    B -- Yes --> E{Group already in DONE?}
    E -- Yes --> F[return false\nIdempotent skip]
    E -- No --> G[Insert group into DONE\nreturn true]
    G --> H[Run subscriber registration block]
    H --> I[SubscriptionHandle forgotten\nLive for process lifetime]
    D --> J[Next bootstrap call retries]
    J --> B

Loading

_{Reviews (2): Last reviewed commit: "fix(core): keep learning subscriber regi..." | Re-trigger Greptile}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e008bc24f

ℹ️ 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".

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/jsonrpc_tests.rs (1)

184-192: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Make the production-wrapper test independent of global mutex state.

group_first_time uses process-global group_first_time_when_bus_ready(...completed...) state. This wrapper test asserts group_first_time(DomainGroup::Media) returns true, but any preceding test that enables the Media group can consume the token and make this test fail under parallel execution. Use the isolated group_first_time_when_bus_ready fixture with an explicit bucket, or add mutex reset/exposure in the test helper for this production wrapper path.

🤖 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/core/jsonrpc_tests.rs` around lines 184 - 192, Update
domain_subscriber_registration_wrapper_uses_the_global_bus to avoid relying on
shared group_first_time_when_bus_ready completed-state: use the isolated fixture
with an explicit bucket, or reset/expose the test mutex state before exercising
group_first_time(DomainGroup::Media). Preserve the assertions that the first
registration succeeds and the second returns false.

🤖 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/core/jsonrpc_tests.rs`:
- Around line 184-192: Update
domain_subscriber_registration_wrapper_uses_the_global_bus to avoid relying on
shared group_first_time_when_bus_ready completed-state: use the isolated fixture
with an explicit bucket, or reset/expose the test mutex state before exercising
group_first_time(DomainGroup::Media). Preserve the assertions that the first
registration succeeds and the second returns false.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 38528ce4-0d45-4ede-abe1-a2a0d5546db0

📥 Commits

Reviewing files that changed from the base of the PR and between 3e008bc and 8e4eed1.

📒 Files selected for processing (2)

• src/core/jsonrpc.rs
• src/core/jsonrpc_tests.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/core/jsonrpc.rs

[samrusani]

Reviewed the outside-diff suggestion about the DomainGroup::Media wrapper test. A repository-wide call-site check shows group_first_time(DomainGroup::Media) is used only by that test; the production registrar has no Media registration block, so enabling the Media domain cannot consume this token. The isolated helper tests already cover the pure state transitions, while this test intentionally covers the real global-bus wrapper. I am keeping it unchanged. CodeRabbit's follow-up review is approved and all review threads are resolved.