Skip to content

Hosts reach onboarding through runtime.account_setup() (#2782 PR 2) - #3270

Merged
erskingardner merged 1 commit into
masterfrom
step7-pr2-account-setup-accessor
Nov 10, 2026
Merged

erskingardner merged 1 commit into
masterfrom
step7-pr2-account-setup-accessor

Conversation

@erskingardner

@erskingardner erskingardner commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Problem

After #3266, Rust hosts still start and drive onboarding through runtime.accounts(), and AccountManager carries twenty one-line forwards into account setup only to serve them. That keeps a hand-written layer on the type that should own only the account workers.

Fix

AccountSetup becomes public, and MarmotAppRuntime::account_setup() returns it, pairing the runtime's setup state with its account manager. The twenty forwards are deleted and their methods keep their signatures and docs on AccountSetup. AccountManager exposes no onboarding method in normal builds; its own account_setup() is test-only.

marmot-uniffi's onboarding commands and the tests change only the path, from runtime.accounts().begin_onboarding(..) to runtime.account_setup().begin_onboarding(..). marmot-c goes through marmot-uniffi and has no source change. Runtime-level setup methods such as login and create_identity stay on MarmotAppRuntime. Nothing changes behaviour.

Verification

  • Local: cargo fmt --all; cargo clippy --workspace --all-targets --all-features and cargo clippy -p marmot-app -p marmot-uniffi --all-targets, -D warnings
  • Local: cargo nextest run -p marmot-app --features test-policy-overrides over the onboarding suite, the relay_runtime onboarding tests, generated setup, setup readiness and sign-out: 146 passed; marmot-uniffi onboarding: 4 passed
  • Local: the UniFFI Swift, Kotlin and Python bindings generated from master and from this branch are byte-identical, and just c-header leaves marmot.h unchanged
  • Local: just binding-docs-gate (only source-link line anchors in the uniffi API reference move), just c-parity-gate, just naming-gate, just tracing-audit, and the land-change smoke (simulator and wn/wnd)
  • Review: Codex and Cursor recommend merge at 9c4fb21. Rebased onto master after Make AppClient public only under test-support (#2782 step 7 PR 3) #3268 for a changelog-only conflict (both entries kept; the code patch is unchanged), then re-ran the scoped clippy and the covering tests

The marmot-app changelog lists the twenty moved methods. Tests change only the path to these methods, plus four bindings that keep a borrowed AccountSetup alive across a held future.

Part of #2782

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Warning

Review limit reached

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Next included review available in 45 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 82 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 4be866eb-13cb-4870-be45-d074f54f8ee0
Commits

Reviewing files that changed from the base of the PR and between 9c4fb21 and 7a84049.

Files selected for processing: 10

  • crates/marmot-app/AGENTS.md
  • crates/marmot-app/CHANGELOG.md
  • crates/marmot-app/src/lib.rs
  • crates/marmot-app/src/runtime/mod.rs
  • crates/marmot-app/src/runtime/onboarding.rs
  • crates/marmot-app/src/runtime/onboarding/tests.rs
  • crates/marmot-app/src/tests/device_group/sign_out.rs
  • crates/marmot-app/tests/relay_runtime.rs
  • crates/marmot-uniffi/API-REFERENCE.md
  • crates/marmot-uniffi/src/commands/onboarding.rs

No actionable comments were generated in the recent review.

Walkthrough

The PR exposes onboarding operations through MarmotAppRuntime::account_setup() and removes the corresponding host-facing forwarding methods from AccountManager. UniFFI commands and onboarding tests now use the runtime accessor.

Changes

Onboarding API

Layer / File(s) Summary
Runtime onboarding entry point
crates/marmot-app/src/runtime/mod.rs, crates/marmot-app/src/runtime/onboarding.rs, crates/marmot-app/src/lib.rs, crates/marmot-app/AGENTS.md, crates/marmot-app/CHANGELOG.md
AccountSetup is public and available through MarmotAppRuntime::account_setup(). The runtime readiness lookup uses this accessor, and AccountManager no longer exposes the onboarding forwarding methods. Public exports and onboarding guidance are updated.
UniFFI onboarding delegation
crates/marmot-uniffi/src/commands/onboarding.rs, crates/marmot-uniffi/API-REFERENCE.md
UniFFI onboarding commands use runtime.account_setup() without changing command arguments, return conversions, or async behavior. The API reference source links are updated.
Onboarding test coverage
crates/marmot-app/src/runtime/onboarding/tests.rs, crates/marmot-app/tests/relay_runtime.rs, crates/marmot-app/src/tests/device_group/sign_out.rs
Onboarding tests use the runtime accessor. A test verifies that canceling ready onboarding twice signs out the account, removes its snapshot, and stops its managed account.

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

Possibly related issues

  • marmot-protocol/mdk#2782 — The issue calls for moving onboarding access out of AccountManager and exposing it through runtime.account_setup(), which this PR implements.

Possibly related PRs

  • marmot-protocol/mdk#3266 — This PR completes the onboarding extraction by adding the runtime accessor and removing the AccountManager forwarding methods.
  • marmot-protocol/mdk#2619 — This PR exposes the existing AccountSetup abstraction through the runtime and removes its AccountManager host forwards.
Merge Risk: Minimal · previous review 9c4fb

This refactor moves onboarding access from the account manager to a runtime accessor without changing behavior. It is a breaking Rust API change, but the changelog documents it. No concrete merge-blocking risk was identified.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 15.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: hosts access onboarding through runtime.account_setup().
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 15.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 59 functions across 6 files. (4 skipped: 3 unsupported, 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Team
  • Run ID: b6956a84-a8e8-4ac6-a7cf-c6b96c47a93f
Commits

Reviewing files that changed from the base of the PR and between ca487ba and 9c4fb21.

Files selected for processing: 10

  • crates/marmot-app/AGENTS.md
  • crates/marmot-app/CHANGELOG.md
  • crates/marmot-app/src/lib.rs
  • crates/marmot-app/src/runtime/mod.rs
  • crates/marmot-app/src/runtime/onboarding.rs
  • crates/marmot-app/src/runtime/onboarding/tests.rs
  • crates/marmot-app/src/tests/device_group/sign_out.rs
  • crates/marmot-app/tests/relay_runtime.rs
  • crates/marmot-uniffi/API-REFERENCE.md
  • crates/marmot-uniffi/src/commands/onboarding.rs

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

After PR 1, hosts still called onboarding through twenty one-line forwards
on AccountManager, so the manager kept a hand-written layer of setup methods.

AccountSetup becomes public and MarmotAppRuntime::account_setup() returns
it, pairing the runtime's SetupState with its AccountManager. The forwards
are deleted; AccountManager keeps its account_setup() only for unit tests.
marmot-uniffi and the tests call the same methods through the new accessor.
The UniFFI and C bindings do not change.

Part of #2782

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@erskingardner
erskingardner force-pushed the step7-pr2-account-setup-accessor branch from 9c4fb21 to 7a84049 Compare November 10, 2026 10:24
@erskingardner
erskingardner added this pull request to the merge queue Nov 10, 2026
Merged via the queue into master with commit d9e54ee Nov 10, 2026
49 checks passed
@erskingardner
erskingardner deleted the step7-pr2-account-setup-accessor branch November 10, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant