Skip to content

feat(identity): operate-as screens obey the app-scoped selected identity (W2–W5) - #868

Merged
lklimek merged 11 commits into
feat/identity-hub-implfrom
feat/app-scoped-selection-migration
Jul 1, 2026
Merged

feat(identity): operate-as screens obey the app-scoped selected identity (W2–W5)#868
lklimek merged 11 commits into
feat/identity-hub-implfrom
feat/app-scoped-selection-migration

Conversation

@lklimek

@lklimek lklimek commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Why this PR exists

What was done

Per the committed plan in docs/ai-design/2026-06-30-app-scoped-selection-migration/01-migration-plan.md, every identity-input site was classified and migrated by responsibility:

  • 12 SYNC-on-change screens now seed their identity from resolve_selected_identity() on entry (fallback first()) and write a user's pick back via set_selected_identity (through IdentitySelector::syncing_global), so switching once propagates everywhere: register/update data contract, document action, DPNS registration, the token creator, and 7 DashPay screens (add contact, contacts list, contact requests, payment history, profile, QR generator, QR scanner).
  • 3 READ-only screens (create-asset-lock top-up, GroveSTARK tool, wallet send) seed the selection as a default but deliberately do not write back — they are wallet-primary or capability-filtered, so a write-back would reconcile the global wallet and fight the screen's own wallet.
  • GroupActions stays session-local (multi-signer ambiguity), regression-locked.
  • Recipient/target pickers are untouched and regression-locked: mint/transfer/freeze/unfreeze/destroy token screens, token control-group members, and the identity transfer recipient name a different party, not you. A test asserts none of them can ever hijack the global active identity.

wallet follows identity: no operate-as screen reads selected_wallet_hash for signing; set_selected_identity reconciles the derived wallet (clearing it for a wallet-less identity).

User story

IDH-003: Multi-identity switching flips from [Gap] to [Implemented]:

As Priya, with multiple wallets and identities, I switch between them from the breadcrumb pill on any tab, and every screen I then open operates as the identity I picked. Recipient and target pickers (sending, freezing, transferring to someone else) leave my active identity unchanged.

Testing

  • cargo test --all-features --workspace1225 lib + 183 kittest, 0 failures.
  • cargo clippy --all-features --all-targets -- -D warnings — clean.
  • cargo +nightly fmt --all — clean.
  • Adversarial QA pass: verdict SHIP, no correctness defects. The IdentitySelector partition (12 SYNC + 1 with_app_default + 9 inert) was confirmed against the existing default_selector_has_no_sync_target invariant, and the double-default trap was handled in all four affected DashPay screens.
  • Keystone write-back is locked at two levels: a unit test calling sync_to_global() directly, and an egui-kittest (combo_change_writes_selection_to_app_context) that drives the real ComboBox Alice→Bob and asserts the global moves — while asserting that seeding alone never writes back.

Breaking changes

None user-facing. Transitively inherits the wallet-DB re-sync introduced by the platform-wallet migration (via #842).

Follow-ups

  • A WalletFixture builder to unblock per-screen click-through write-back tests and the loaded-HD-wallet switching kittests (IT-SWITCH-01/02).
  • A positive EdDSA-seed test for GroveSTARK (needs an EdDSA key fixture); the negative R4 guard is already locked.
  • Fold the hub's per-frame identity-table load into one shared snapshot (hub_screen.rs TODO).

Checklist

  • Tests green (lib + kittest), clippy clean, fmt clean
  • Adversarial QA pass (SHIP)
  • User stories updated (IDH-003 → Implemented)
  • Design/migration plan committed under docs/ai-design/
  • CI green (pending)

🤖 Co-authored by Claudius the Magnificent AI Agent

lklimek and others added 11 commits June 30, 2026 07:21
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…/document screens (W2)

Batch B1 of the W2-W5 migration plan. Each screen seeds its initial identity
from `selected_identity_id()` (fallback: first loaded) and adds
`.syncing_global()` to its `IdentitySelector` so a user pick propagates back to
the app-scoped selection.

- `RegisterDataContractScreen::new()`: seed from selected_identity_id
- `UpdateDataContractScreen::new()`: seed from selected_identity_id
- `DocumentActionScreen::new()`: seed via resolve_selected_identity() when None
- All three IdentitySelectors: `.syncing_global(self.app_context.clone())`

Tests: 3 kittests in tests/kittest/contract_screen.rs asserting each screen
defaults to the app-scoped identity on construction (seeding direction).
Write-back direction deferred (TODO: private-key fixture, TI-1).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…(W2)

Batch B2: `RegisterDpnsNameScreen::new()` seeds `selected_qualified_identity`
from `selected_identity_id()` (fallback: first loaded) so the DPNS registration
screen opens on the identity the user last operated as, not always the first DB
row.

The `IdentitySelector` in `render_identity_id_selection` now carries
`.syncing_global()` so a user pick writes back to the app-scoped selection.

Test: extended `tests/kittest/register_dpns_name_screen.rs` with
`dpns_registration_defaults_to_app_scoped_identity`, which seeds two identities,
sets the second as the global selection, constructs the screen, and asserts it
opens on the second identity.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tity (W3)

Batch B3: all 7 DashPay screens now seed `selected_identity` from the app-scoped
selection on construction and write user picker changes back via `syncing_global`.

Screens migrated (seed in `new()` + `refresh()`; `syncing_global` on selector):
- `AddContactScreen::new()` + `new_with_identity_id()` — seeded; selector syncs
- `ContactsList::new()` + `refresh()` — seed prefers scoped id over first
- `ContactRequests::new()` + `refresh()` — seed prefers scoped id over first
- `PaymentHistory::new()` + `refresh()` — seed prefers scoped id over first
- `ProfileScreen::new()` + `refresh()` — seed prefers scoped id over first
- `QRCodeGeneratorScreen::new()` — seeded; selector syncs
- `QRScannerScreen::new()` — previously `None`; now seeds from scoped id

`selected_identity` made `pub` on each struct for test verification (consistent
with `RegisterDataContractScreen` / `DocumentActionScreen` precedent).

Tests (9 assertions in `tests/kittest/dashpay_screen.rs`): each screen opens on
the second of two seeded identities when it is set as the app-scoped selection.
Write-back canary deferred (TI-1 / private-key fixture gap; TODO added).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Batch B4: `TokensScreen::new()` seeds `selected_identity` from the app-scoped
identity on construction (falling back to the first loaded identity).

Changes:
- `mod.rs`: after the struct literal, look up the preferred id in the already-
  built `identities` BTreeMap and populate `selected_identity` + `identity_id_string`.
- `token_creator.rs` (simple mode): added `.syncing_global()` to the
  `IdentitySelector` so a user pick writes back to the app-scoped selection.
- `token_creator.rs` (advanced mode): snapshot before/after `add_identity_key_chooser`
  and call `set_selected_identity` on change (helper uses raw ComboBox, not
  IdentitySelector, so manual write-back is needed).
- `selected_identity` made `pub` on `TokensScreen` for test verification.

Test: `tests/kittest/tokens_screen.rs` —
`token_creator_defaults_to_app_scoped_identity` seeds two identities, sets the
second as the global selection, constructs the screen, and asserts it opens on
the second identity. Write-back requires a private-key fixture (TI-1 gap; TODO).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-scoped identity (W5)

Batch B5: three READ-only screens now seed their identity selector from the
app-scoped selection subject to their specific membership guards. No screen
uses `syncing_global` (R1/R4 reasons documented inline).

Changes:
- `create_asset_lock_screen.rs`: Added `.with_app_default(&self.app_context)`
  to the "Identity to top up" selector. Guard: seeds only if the global id is
  in this wallet's identity list (IdentitySelector handles the check). No
  `new()` pre-fill change needed.

- `grovestark_screen.rs`: Manual EdDSA-guarded seed in `new()` and
  `refresh_identities()`. The global identity is used iff it passes the
  EdDSA-key filter; otherwise falls back to first EdDSA identity or `None`.
  `selected_identity` made `pub` for test verification.

- `send_screen.rs`: Manual wallet-membership-guarded seed at render-time: when
  the wallet-scoped identity list is built each frame, seeds `selected_identity`
  from the global id iff it is among this wallet's identities; otherwise
  `selected_identity` stays `None`.

Tests: `tests/kittest/tools_screen.rs` —
`grovestark_does_not_seed_non_eddsa_identity`: seeds two basic identities (no
EdDSA keys), sets the second as the global selection, constructs the screen, and
asserts `selected_identity == None` (R4 guard). Positive-seed and wallet-membership
tests deferred (EdDSA-key fixture and WalletFixture gaps; TODOs in test file).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Batch B6: regression-lock tests and the K3 comment on GroupActionsScreen.

GroupActionsScreen:
- Added one-line K3 comment to the IdentitySelector: session-local screen,
  no `with_app_default` or `syncing_global` by design.
- `selected_identity` made `pub` for test verification.

Tests:
- `contract_screen::group_actions_does_not_seed_from_global_identity` (K3 lock):
  seeds two identities, sets the second as global, creates `GroupActionsScreen`,
  asserts `selected_identity == None` and the global selection is unchanged.
  This pins the session-local behaviour against future drift.
- Updated `tokens_screen.rs` doc-comment to document the B6 N/A regression-lock
  reasoning: the 6 N/A token recipient/target/member selectors are covered by
  the `default_selector_has_no_sync_target` unit test in identity_selector.rs;
  a structural note captures this invariant here.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Formatting-only commit: `cargo +nightly fmt --all` on all files touched during
the B1-B6 migration batches. No functional changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…unit tests; fix QA-002 misleading docstring

QA-001 (MED): add fixture-free `syncing_global_writes_selection_to_app_context` unit
test inside `identity_selector::tests`. Calls `sync_to_global()` directly (private
access in same module) with in-memory `QualifiedIdentity` structs and a bare
`AppContext` (no Harness, no wallet-backend wiring needed — KV persistence
gracefully skips). Proves the write-back path without private keys or DB insertion.

QA-003 (LOW): add `with_app_default_inert_when_global_id_not_in_candidate_list` unit
test. Verifies that when the app-scoped identity is absent from the selector's
candidate list, `app_default_seed()` returns `None` — locks the wallet-membership
guard that `CreateAssetLockScreen` relies on (R1).

QA-002 (LOW): correct the misleading `contacts_list_defaults_to_app_scoped_identity`
docstring in `dashpay_screen.rs`. It called itself a "write-back canary" but only
tests seeding; updated to accurately describe seeding coverage and point to QA-001
for write-back coverage.

QA-004 (LOW): update deferred TODO comments in `contract_screen.rs`, `tokens_screen.rs`,
and `tools_screen.rs` to reference the new QA-001 unit test so readers know write-back
is now covered at the component level.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The app-scoped selected identity now drives every operate-as screen
(W2-W5), completing the multi-identity switching story.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…test

Replace the unit-test-only QA-001 proof with a proper kittest that exercises
the full rendering path: a genuine ComboBox change → `sync_to_global()` →
`AppContext::set_selected_identity()`.

`tests/kittest/identity_selector.rs` — `combo_change_writes_selection_to_app_context`:
- Phase 1: initial render with buffer pre-seeded to Alice must NOT invoke
  `set_selected_identity` (seeding ≠ write-back).
- Phase 2: `get_by_value("Alice").click()` opens the ComboBox popup,
  `get_by_label("Bob").click()` selects Bob; asserts `ctx.selected_identity_id()
  == Some(bob_id)` after `harness.run()`.

Setup pattern: `build_eframe` + `run_steps(5)` to fully wire `ensure_wallet_backend`
(and drain `restore_selected_identity_from_kv`) BEFORE seeding the identity, so the
async initialization race does not overwrite the seed.

Unit test `syncing_global_writes_selection_to_app_context` in `identity_selector.rs`
is kept and re-scoped to the *mechanism* (`sync_to_global()` method). The new kittest
covers the *rendering gate* (`combo_changed || text_response.changed()` at line 321).

Also updates `identity_selector.rs` docstring to cross-reference the kittest.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • master
  • v1.0-dev

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8343acb4-201d-49de-8c0f-8719b1f8a6e8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/app-scoped-selection-migration

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

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

@lklimek
lklimek marked this pull request as ready for review July 1, 2026 10:04
@lklimek
lklimek merged commit 5f3772c into feat/identity-hub-impl Jul 1, 2026
5 checks passed
@lklimek
lklimek deleted the feat/app-scoped-selection-migration branch July 1, 2026 10:04
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