Skip to content

Make AppClient public only under test-support (#2782 step 7 PR 3) - #3268

Merged
erskingardner merged 1 commit into
masterfrom
step7-pr3-appclient-test-support
Nov 10, 2026
Merged

erskingardner merged 1 commit into
masterfrom
step7-pr3-appclient-test-support

Conversation

@erskingardner

Copy link
Copy Markdown
Member

Problem

AppClient, the directly owned per-account client, is still part of marmot-app's public API, so a host can bypass
MarmotAppRuntime and open an account's engine session itself. No host does today: wn sync is the only
production caller, and it opens one only to run one sync pass. Nobody outside the codebase feels this yet.

Fix

AppClient and MarmotApp::client are now public only under test-support, as marmot-session's session has been
since step 5. wn sync calls a new MarmotApp::sync_account, which opens the same client, runs the same pass and
keeps the two failures apart: an open failure stays an AppError, and a pass failure stays a SyncFailure with its
partial summary. wn sync output and exit codes do not change.

Verification

  • Local: cargo fmt --all; cargo clippy --workspace --locked -- -D warnings, then with --all-targets --all-features
  • Local: cargo nextest run -p marmot-app --features test-policy-overrides on the six sync binaries (12/12), the
    44 wn-cli integration tests that run wn sync, the 4 commands::sync unit tests, and -p marmot-app --lib
    (2205/2205)
  • Local: end-to-end smoke (both commands from land-change step 1)
  • Local: just binding-docs-gate, just naming-gate; just c-header leaves marmot.h byte-identical
  • Review: Codex (gpt-6.1-sol) and Cursor (grok-4.7-high-fast) recommend merge at 847f207

Part of #2782 (step 7 PR 3).

Notes

  • Public surface (rustdoc JSON against master): without test-support, it loses the AppClient struct, its 82
    public methods and MarmotApp::client, and gains MarmotApp::sync_account. With test-support or
    --all-features, only sync_account is new. Rustdoc warnings are unchanged. marmot-uniffi and marmot-c are
    untouched, so the bindings and marmot.h are unchanged.
  • Test selection: marmot-app lists the same 2433 tests with default features. With test-policy-overrides it
    lists one more, the new sync_account test. A self dev-dependency enables test-support, so -p marmot-app runs
    still build every integration test.
  • cargo tree: wn-cli, marmot-uniffi, marmot-c and agent-connector enable test-support only through dev
    edges. Since Recovery runner owns account history by default (#2660 phase 4) #2950 the conformance simulator depends on marmot-app with test-support as a normal dependency,
    and incident-replay and the campaign runner inherit it from the simulator. None of the three uses AppClient.
  • 29 AppClient methods that only tests call carry #[cfg_attr(not(feature = "test-support"), allow(dead_code))].
    11 of them have no caller in any build. Deleting them is left as a follow-up so the test-support surface stays
    unchanged here.
  • sync_account boxes the pass: one more future layer otherwise exceeded the compiler's layout query depth when it
    computed wn's command future.
  • Open PRs touching the same files: Move idle maintenance onto per-account wakes and deadlines (mdk#2923 step B) #3258 (mdk#2923) and Account setup out of AccountManager (#2782 PR 1) #3266 (Step 7 plan: inside marmot-app #2782 PR 1). A trial merge with each is clean.

🤖 Generated with Claude Code

Hosts reach accounts through MarmotAppRuntime. `AppClient` and
`MarmotApp::client` are now public only under `test-support`, and `wn sync`
runs its one pass through the new `MarmotApp::sync_account`, which keeps an
open failure (`AppError`) apart from a pass failure (`SyncFailure` with its
partial summary).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

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 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 81 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: 3d52509d-7256-4ddb-9a45-c3ed3e8720f4
Commits

Reviewing files that changed from the base of the PR and between 67fde52 and 847f207.

Files ignored due to path filters: 1

  • Cargo.lock is excluded by !**/*.lock
Files selected for processing: 9

  • crates/cli/src/commands/sync.rs
  • crates/marmot-app/AGENTS.md
  • crates/marmot-app/CHANGELOG.md
  • crates/marmot-app/Cargo.toml
  • crates/marmot-app/src/client/mod.rs
  • crates/marmot-app/src/client/sync.rs
  • crates/marmot-app/src/error.rs
  • crates/marmot-app/src/lib.rs
  • crates/marmot-app/tests/partial_sync_summary.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@erskingardner
erskingardner added this pull request to the merge queue Nov 10, 2026
Merged via the queue into master with commit b7f00a6 Nov 10, 2026
53 checks passed
@erskingardner
erskingardner deleted the step7-pr3-appclient-test-support branch November 10, 2026 10:12
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