Repository navigation
feat(server): add admin-only RenameAccount RPC (RIG-5151) - #2226
Merged
Merged
Conversation
Handle rename had no write path: the only rename was a raw UPDATE in tests. RenameAccount gives the admin an in-place rename for users and agents, following DL-271: rename in place at both tiers, and a freed handle may be reclaimed at once. Store.RenameHandle updates account_handles (the resolution key) and accounts.handle (the display copy) in one tenant transaction. A unique violation maps to ErrConflict, which the handler returns as AlreadyExists. The handler resolves the account as IssueToken does, refuses the system account, and returns an agent's address owner-qualified. The AdminGate classifies the RPC adminOnly. The product spec gains the requirement and its scenarios. User self-rename is out of scope and stays with its own design. Ledger-impact: none Co-authored-by: Matt Wilkinson <matt@rigel.build>
… rename (RIG-5151) Review follow-up. Same-owner DM names are built from handles, so after a rename a reclaimed handle resumed the old DM and its history. RenameHandle now re-keys every same-owner DM of the renamed account (agent-agent and owner-agent) in the rename transaction, under the owner's DM lock, and OpenDM reads both handles inside that lock. A rename publishes AccountChanged and a ChannelChanged per re-keyed DM. The bootstrap admin and its root supervisor are refused with FailedPrecondition, because startup finds them by handle. Ledger-impact: none Co-authored-by: Matt Wilkinson <matt@rigel.build>
…G-5151) Review follow-up. An agent's home channel and a manager's coordination channel are named from the handle, so a rename left them stale. A later reconcile would then create a second coordination channel. RenameHandle now renames both in the same transaction, keeping ids, with the existing collision suffix for coordination names. The coordination reconcile re-reads the manager handle after taking the owner lock, so a queued reconcile cannot bring back the old name. DM re-keying now matches only real DM parties, not owners pulled into agent-agent DMs. It skips a DM converted to a channel after the scan. Ledger-impact: none Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
😎 This pull request was merged. |
Contributor
Author
|
Review loop at head cfad468. Round 1 found 1 high and 2 medium: DM history reachable through a reclaimed handle, no AccountChanged event, and built-in identities renamable. Round 2 found 2 medium: a race with DM-to-channel conversion, and stale home and coordination channel names. Round 3 found 1 medium and 1 low: an owner-handle overlap re-keying an agent-agent DM, and a missing coordination event test. All are fixed with regression tests and mutation checks. The final interdiff review found 0/0/0. |
|
Compass engineering docs preview: https://compass-repo-rig-5151-rename.compass-eng-docs.pages.dev Deployed from Changed pages: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add an admin-only
RenameAccountRPC. It renames a user or an agent in place, following DL-271: rename in place at both tiers, and a freed handle may be reclaimed.Why
Matt ruled B on handle renames: an admin RPC now, with user self-rename later as its own design (RIG-4581). Before this there was no rename write path; only tests renamed handles, with raw SQL.
Change
Store.RenameHandlechanges these in one tenant transaction:account_handles.handle(the resolution key) andaccounts.handle(the display copy);A taken handle is
ErrConflict. Locks: the owner's DM lock, then the coordination lock. A DM converted to a channel after the scan is skipped.OpenDMreads both handles under the DM lock. The coordination reconcile re-reads the manager handle after taking its lock. So a queued open or reconcile cannot bring back the old name.CompassService.RenameAccountis classifiedadminOnlyin the AdminGate. It resolves the account the wayIssueTokendoes. Errors:new_handleInvalidArgumentNotFoundAlreadyExistsPermissionDeniedFailedPreconditionIt returns the new address, owner-qualified for agents. After commit it publishes
AccountChanged, plus aChannelChangedfor each renamed channel.Proto, generated Go and TS clients, and the product spec are updated (new requirement and scenarios; admin-only lists).
Verification
AccountChangedandChannelChangedfor home, coordination and DM channels;internal/comms,internal/store,serverandinternal/authpgtest packages pass.golangci-lintreports 0 issues on both tag sets. Nilaway, proto lint/drift and sqlc-drift are clean, and rumdl is clean on the spec.Refs RIG-5151.
Ledger-impact: none (implements DL-271's rename policy; no new decision).