fix(tinyplace): wire handle transfer end-to-end (Closes #4929)#4998
fix(tinyplace): wire handle transfer end-to-end (Closes #4929)#4998M3gA-Mind wants to merge 86 commits into
Conversation
The SDK exposed registry.transfer (POST /registry/names/{name}/transfer)
but nothing above it reached the method: no core controller, no client
method, no UI. Handle transfer was unreachable from the app (tinyhumansai#4929).
Bridge every layer:
- core: handle_tinyplace_registry_transfer resolves the recipient @handle
to its cryptoId + publicKey via registry.get, calls registry.transfer,
and read-back-confirms the returned identity is now owned by the
recipient (fails closed on unresolved recipient / missing key material /
mismatch). Registered via schemas.rs + the controller registry. No PII
logged (status-only debug lines).
- client: apiClient.registry.transfer(name, recipient) + TransferHandleResult.
- UI: a per-handle Transfer action on non-primary owned handles in the
Profiles card opens TransferHandleModal, a destructive-confirm dialog
that requires a recipient + explicit confirm and fails closed on error.
- i18n: seven agentWorld.transferHandle.* keys across en + all 13 locales
(no em dashes).
- tests: TransferHandleModal (wiring, recipient-gating, fail-closed),
ProfilesSection (Transfer opens the modal), and a Rust handler test for
the name/recipient validation branches.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughAdds end-to-end handle transfer support: a validated and registered Rust RPC, an SDK client method, a localized profile modal for non-primary handles, success/error handling, and frontend/core tests. ChangesHandle transfer flow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
bug
rust-core
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 143512ca05
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@app/src/agentworld/components/TransferHandleModal.tsx`:
- Around line 55-59: Update the catch block in the transfer handler around the
debug call to log only a fixed failure-state message, removing String(err) and
any raw error details from logging. Preserve the existing setError(String(err))
UI handling and setSubmitting(false) behavior unchanged.
In `@app/src/agentworld/pages/ProfilesSection.tsx`:
- Around line 379-389: Update the server-side
handleTransfer/handle_tinyplace_registry_transfer handler to identify whether
the requested name is a primary handle and reject the request before calling
registry.transfer. Preserve transfers for non-primary handles, and add a test
covering rejection of a primary-handle transfer through the JSON-RPC path.
In `@app/src/lib/i18n/bn.ts`:
- Around line 467-468: Update the Bengali translation for
agentWorld.transferHandle.warning so the recipient clause uses “প্রাপক একমাত্র
মালিক হয়ে যাবেন।” instead of the grammatically awkward wording, while
preserving the rest of the warning.
In `@app/src/lib/i18n/pl.ts`:
- Around line 476-483: Update the Polish ownership-transfer translations in the
agentWorld.transferHandle entries to use “Przekaż handle” and “Przekazanie
handle’a...” wording consistently for the action, title, warning, and
confirmation labels; leave the recipient, submitting, and required-field
messages unchanged.
In `@src/openhuman/tinyplace/manifest.rs`:
- Around line 5362-5382: The test transfer_requires_name_and_recipient only
covers input validation; extend the transfer test coverage around
handle_tinyplace_registry_transfer to include name "@" normalization, mocked
recipient key resolution, SDK transfer failure, successful confirmation, and
ownership mismatch. Add assertions for each outcome while keeping network and
destructive operations mockable.
- Around line 2513-2578: Move handle_tinyplace_registry_transfer from
manifest.rs into schemas.rs, keeping it as the RPC boundary and changing its
return type to RpcOutcome<T>. Extract recipient lookup, transfer, and read-back
confirmation into a dedicated ops.rs operation, then have the handler validate
RPC parameters, call that operation, and serialize its result without retaining
business logic in the controller.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b0dd2c0-b461-4153-befd-ceac0b5f512f
📒 Files selected for processing (21)
app/src/agentworld/components/TransferHandleModal.test.tsxapp/src/agentworld/components/TransferHandleModal.tsxapp/src/agentworld/pages/ProfilesSection.test.tsxapp/src/agentworld/pages/ProfilesSection.tsxapp/src/lib/agentworld/invokeApiClient.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tssrc/openhuman/tinyplace/manifest.rssrc/openhuman/tinyplace/schemas.rs
…4929-tinyplace-handle-transfer
- schemas.rs: restore rustfmt wrapping of the registry_transfer 'name' input (fixes the Rust Quality + Frontend Checks fmt lanes). - TransferHandleModal: drop String(err) from the failure debug log so no backend/SDK detail is logged (the UI still shows it via setError). - manifest.rs: reject transferring a PRIMARY handle server-side, not just in the UI — a direct JSON-RPC caller could otherwise move the wallet's active identity. Also cover the '@' name-normalization case in the test. - i18n: fix the Bengali warning grammar; use ownership-transfer wording (Przekaż/Przekazanie) in Polish.
|
Pushed 2c089f2 (rebased onto current main). Summary of changes: Red lanes — the sole cause of Frontend Checks, Rust Quality (fmt, clippy), and the PR CI Gate rollup was one rustfmt diff: CodeRabbit review (all 6 addressed):
Codex — fallback-profile reachability noted as a follow-up (the reverse fallback only carries the primary handle, which isn't transferable). @coderabbitai review — re-review after fmt fix + server-side primary-handle guard + PII-safe logging. |
|
✅ Action performedReview finished.
|
…4929-tinyplace-handle-transfer
|
@coderabbitai review Rebased onto current upstream/main (clean merge, only touched tinyflows/caps.rs). All prior CodeRabbit findings are resolved on this head:
No code changes since your last pass — this is a main-sync + re-review request to refresh the stale CHANGES_REQUESTED verdict. |
|
✅ Action performedReview finished.
|
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
@M3gA-Mind hey! the code looks good to me — this is a solid, careful piece of work. Wiring registry.transfer through core → client → UI with a resolve-then-verify pattern (recipient resolved via registry.get before signing, read-back confirmation after, fail-closed on every unhappy path) is exactly the right amount of caution for a destructive/irreversible action. The follow-up commit adding the server-side primary-handle guard (so a direct JSON-RPC caller can't orphan the wallet's active identity by bypassing the UI check) was a good catch on your own part. PII-safe logging, i18n across all 13 locales, and the fail-closed modal all check out.
Two very minor things, not blocking:
handle_tinyplace_registry_transfernow makes three sequential network round-trips (get-own-identity for the primary check, get-recipient, then transfer). There's a small TOCTOU window between the primary check and the actual transfer call — low real-world impact since only the owning wallet's own signature can execute the transfer, but worth being aware of if this pattern gets reused elsewhere.- In
TransferHandleModal.tsx, thedebug()calls already run under theagentworld:identitynamespace, so the[agentworld:identity]prefix baked into each message string is redundant. Cosmetic only.
On the test-coverage gap CodeRabbit flagged (transfer execution / confirmation-failure paths not covered by the current unit test) — I saw you and CodeRabbit already agreed to defer that to mock-backend JSON-RPC E2E coverage tracked under #4776, so I won't re-litigate it here.
CI is still showing Rust Core Coverage and Frontend Checks as pending — I'll come back once those are green. Nice work on this one.
|
@coderabbitai review All review threads are now resolved on the current head (96bb836) — the earlier CHANGES_REQUESTED is fully addressed:
No code changes needed since your last pass; requesting a re-review to refresh the stale verdict. |
|
✅ Action performedReview finished.
|
coderabbitai
Bot
added
agent
oxoxDev
left a comment
oxoxDev
left a comment
There was a problem hiding this comment.
Transfer is correctly authorized end-to-end. Authz rests on the owner's signature over the canonical identity.transfer payload (SDK registry.rs), not on any caller-supplied param — there's no way to transfer a handle the active wallet doesn't own. Recipient is resolved and fail-closed if unregistered or missing key material; the post-transfer read-back rejects on a crypto_id mismatch; the server-side primary-handle lock closes the direct-JSON-RPC bypass of the UI guard. Modal fails closed (keeps the dialog open on error, close disabled in-flight). i18n 14/14.
Non-blocking:
- Rust coverage only exercises the input-validation branch; the security-relevant branches (primary-lock, recipient resolution, read-back mismatch) are uncovered because the inline module can't mock the client. Deferred to #4776 — reasonable, but worth an E2E on a destructive path.
- Nit: a "type the handle to confirm" step would harden the irreversible action beyond the free-text recipient + danger button.
LGTM.
Addresses the maintainer reviews on tinyhumansai#4998: - oxoxDev (confirmation-UX hardening for the irreversible action): add a 'type the handle to confirm' step to TransferHandleModal — the danger button stays disabled until the user re-types the exact handle (case- and @-insensitive), and submit() fails closed if it doesn't match. Adds agentWorld.transferHandle.confirmLabel / confirmMismatch across en + all 13 locales, plus test coverage for the gate. - graycyrus (cosmetic): the debug() calls already run under the 'agentworld:identity' namespace, so drop the redundant '[agentworld:identity]' prefix baked into each message string.
8fc1e71
|
Thanks @graycyrus and @oxoxDev — addressed the review feedback in 8fc1e71: Fixed
Acknowledged, no change
@graycyrus — could you take another look when the re-run is green? This should clear your two notes. |
coderabbitai
Bot
removed
the
rust-core
…ansai#4947 Bug 2 secret-guard sync failure) (tinyhumansai#4953)
…cle, not isRunning (tinyhumansai#5010)
…urces (Closes #4957) (tinyhumansai#4993)
# Conflicts: # app/src/agentworld/pages/ProfilesSection.tsx # app/src/lib/i18n/ar.ts # app/src/lib/i18n/bn.ts # app/src/lib/i18n/de.ts # app/src/lib/i18n/en.ts # app/src/lib/i18n/es.ts # app/src/lib/i18n/fr.ts # app/src/lib/i18n/hi.ts # app/src/lib/i18n/id.ts # app/src/lib/i18n/it.ts # app/src/lib/i18n/ko.ts # app/src/lib/i18n/pl.ts # app/src/lib/i18n/pt.ts # app/src/lib/i18n/ru.ts # app/src/lib/i18n/zh-CN.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1adfc287ea
ℹ️ 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".
| .get(&format!("@{name}")) | ||
| .await | ||
| .map_err(map_err)?; | ||
| if own.identity.as_ref().and_then(|i| i.primary) == Some(true) { |
There was a problem hiding this comment.
When registry.get returns an identity with primary omitted/null, this check falls through because it only rejects Some(true). The frontend types make primary optional and ProfilesSection.pickPrimary() already treats the first/only unmarked handle as the active profile handle, so in that scenario a direct RPC caller, and the new Transfer button when GraphQL omits the flag, can transfer the active/only handle even though this handler says active handles must be blocked to avoid orphaning the wallet; fail closed unless the handle is explicitly known to be non-primary or verify against the wallet's full identity list.
Useful? React with 👍 / 👎.
7 participants
Summary
registry.transferbut nothing above it reached the method, so a user could not transfer a@handlefrom the app at all.tinyplace_registry_transfer→invokeApiClientregistry.transfer(name, recipient)→ a per-handle Transfer action in the Profiles card that opens a destructive-confirm modal.registry.transfer, and verifies the returned Identity is now owned by the recipient before reporting success. It fails closed on an unresolved recipient, missing key material, or a read-back mismatch.agentWorld.transferHandle.*useT()keys added toen.ts+ all 13 locales (no em dashes); grep-friendlyagentworld:identity/registry_transferlogging with no PII.Problem
The capability existed only in the vendored SDK and was un-bridged at every layer:
vendor/tinyplace/sdk/rust/src/api/registry.rs:254—pub async fn transfer(&self, name, IdentityTransferRequest) -> Result<Identity>(POST/registry/names/{name}/transfer).handle_tinyplace_registry_transferinmanifest.rs, not registered inschemas.rs.registry.transferininvokeApiClient.ts, no transfer action in the UI.Result: transfer could not be initiated from the app. (Established while auditing #4920 —
registry.transferexists, unlike the seller-side marketplace methods, so this is a fixable in-repo wiring gap, not a contract block.)Solution
src/openhuman/tinyplace/manifest.rs):handle_tinyplace_registry_transfer(name, recipient)resolvesrecipientviaregistry.get("@…")(the proven availability path), extracts the recipient'scryptoId+publicKey, buildsIdentityTransferRequest, and callsclient.registry.transfer(&name, req). The SDK attaches the owning wallet's signature, so a caller can only transfer a handle their own wallet owns. Read-back: if the returned identity'scryptoIdis not the recipient's, it returns an error instead of a false success. Registered the controller inschemas.rs(schema + import + all-schemas vec +RegisteredController) — no ad-hoccli.rs/jsonrpc.rsbranch.invokeApiClient.ts):registry.transfer(name, recipient)+TransferHandleResult.ProfilesSection.tsx+ newcomponents/TransferHandleModal.tsx): non-primary owned handles get a Transfer button (a primary handle is locked from transfer). The modal states the action is permanent, requires an explicit recipient and confirm click, disables close while in-flight, and fails closed — on error it keeps the dialog open with the message and never callsonTransferred. Treated with the care of the Emergency-Stop work in feat(safety): Emergency Stop for desktop automation (#4255) #4600.Submission Checklist
TransferHandleModal.test.tsx(confirm callsregistry.transfer('alpha','bravo')+ closes on success; fails closed on error),ProfilesSection.test.tsx(Transfer opens the confirm modal), and a Rusttransfer_requires_name_and_recipienthandler test.schemas.rsregistration by the existingall_controller_schemastests.pnpm test:coverage/test:rustnot run locally (machine limit); CI authoritative.## Related— N/A: no matrix feature ID for this action.registry.transferonly.Closes #NNNin the## Relatedsection.Impact
Related
registry.transfer.)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/GH-4929-tinyplace-handle-transfer143512ca05b915323dd97ea57cd01d19e6a297ddValidation Run
pnpm --filter openhuman-app format:check— N/A: ran targetedprettier --writeon touched TS files; Rust formatting matched against sibling handlers (cargo fmt not runnable locally, fmt lane authoritative).pnpm typecheck— clean.pnpm i18n:checkandpnpm i18n:english:checkrun locally → both exit 0.registry_*handlers, fmt/clippy lanes authoritative.Validation Blocked
command:cargo test/pnpm test:coverageerror:not run — machine constraint (local Rust/JS builds fill disk).impact:unit, coverage, fmt, and clippy verified by CI.Behavior Changes
@handleto another tiny.place identity from the Profiles card.Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
@normalization and a “type handle to confirm” safeguard.