Self-hosted: name the extension that was actually asked to sign - #1360
Conversation
A failed config save said "Keychain signing was cancelled." regardless of which wallet signed. `keychain` is the login type for every Hive browser extension, not the Keychain product, so the same branch runs for Hive Keeper and Peak Vault. A Keeper user was being sent to check a wallet they never installed, and this app leads with Keeper in both its install list and its detection order, so the wrong name was most likely shown to the users it was most wrong for. The message now names the extension recorded for the session, and stays generic when none is, since the preference is stored per username and a browser that cleared storage still signs. Built as a pure exported function so it can be asserted directly: nothing in a .tsx file is testable under this runner, and login-method copy has already drifted from what the code does once. VALID_EXTENSION_IDS is exported so the test covers every wallet rather than a list that can fall behind a fourth one.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (3)
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. Comment |
Two review points on the previous commit. VALID_EXTENSION_IDS was hand-written, so a fourth id added to HiveExtensionId and forgotten here would typecheck fine and leave every caller, including the signing-copy test, quietly covering only the ids someone remembered. It is now derived from EXTENSION_META, which is a Record keyed by HiveExtensionId and so already forced to carry an entry per id. The exhaustiveness is structural rather than remembered. hosting-token-copy.test.ts read as a copy of a hosting-token.test.ts. No such file existed, so the conventional name was free and a later test for token caching would have forced a rename.
1 participant
From the review of the extension flow. Most of what was asked for is already built, in both apps: detection via the Hive Unified Wallet Protocol registry with legacy per-global fallback, a choice when more than one wallet is present, the choice remembered per username, install links led by Hive Keeper, and re-detection on focus for extensions that inject after mount.
ExtensionLoginsays "Sign with extension" andconstants.tssays "Sign with Hive Keeper, Keychain or Peak Vault".One string was not generic.
getHostingToken'skeychaincase is the login type for every Hive browser extension, not the Keychain product. Hive Keeper and Peak Vault both run that branch. When the signing request came back empty it said:So a Keeper user was told to go and check a wallet they never installed. This app leads with Keeper in both its install list and its detection order, so the wrong name was most likely shown to exactly the users it was most wrong for.
Which branch this actually is
Worth stating precisely, because an earlier draft of this description overclaimed it. This is not the common cancel path.
keychain.ts:104rejects withresp.error || 'Operation cancelled'wheneversuccessis false, and Peak Vault cancels throw, so a typical cancel surfaces the extension's own text and never reaches the empty-result check. The message here fires when a wallet resolves success with an empty result.The old string was wrong on that branch regardless, and neither rejection path names a wrong wallet, so this completes the job for this call site. It just is not the most-travelled path, only the one that was lying.
Changes
extensionCancelledMessage(extension)names the wallet recorded for the session, and stays generic when none is. That case is real: the preference is stored per username, so a browser that cleared storage still signs..tsxis testable under this runner (environment: 'node',include: ['src/**/*.test.ts']), and login-method copy has already drifted from what the code does once, in Self-hosted: say the Hivesigner client id is filled in for hosted blogs #1357.VALID_EXTENSION_IDSis now derived fromEXTENSION_META, aRecord<HiveExtensionId, ...>the type checker already forces an entry per id into. A hand-written list would typecheck while missing a fourth wallet, leaving the test covering only the ids someone remembered.hosting-token-copy.test.tsrenamed tohosting-token.test.ts, which was free since no such file existed.Verification
config.jsonerror.Not changed, tracked separately
Three other surfaces name Keychain to users of every wallet, and one residual case where the message can still name the wrong one after a mid-session uninstall.