Skip to content

feat(dashnote): preview identity + key-type fitness on WIF paste#75

Merged
thephez merged 5 commits intomainfrom
feat/login-by-key
May 6, 2026
Merged

feat(dashnote): preview identity + key-type fitness on WIF paste#75
thephez merged 5 commits intomainfrom
feat/login-by-key

Conversation

@thephez
Copy link
Copy Markdown
Collaborator

@thephez thephez commented May 5, 2026

Summary

  • Accept a WIF private key as a login secret in addition to a BIP39 mnemonic; secret shape is auto-detected from whitespace, and the identity-index field is hidden when WIF is detected (single-key, no DIP-13 derivation needed).
  • Eagerly resolve the owning identity + DPNS name once the pasted secret passes a cheap structural gate (length 51/52, base58 charset) plus a 400 ms debounce, so users see who the key belongs to before clicking Login.
  • Surface "wrong key type" / "key disabled" warnings pre-submit and disable the Login button — pasting a MASTER, TRANSFER, or ENCRYPTION key (or a disabled key) is now caught before a transition is attempted, with copy that distinguishes "MASTER authentication key" from "TRANSFER key".
  • Split a signer-free resolveIdentityFromWif out of loginWithPrivateKey so the preview path can validate without touching the WASM signer; WrongKeyPurposeError now also carries securityLevelName.
  • UnknownIdentityError and transient errors stay silent in the preview (no pre-submit error UI); the actual login still surfaces them on submit.

Summary by CodeRabbit

  • New Features

    • Added private key (WIF) login alongside mnemonic authentication
    • Single "Mnemonic or Private Key" input with automatic secret-type detection
    • UI adjusts for WIF: hides identity index, updates button labels, shows informational note
    • Real-time WIF validation preview and disabled login when input is invalid
    • Session login API accepts a secret and better preserves/restores state on failures
  • Tests

    • Comprehensive tests for WIF flows, detection logic, preview hook, and session behavior

thephez and others added 2 commits May 5, 2026 17:17
…t shape

Login form now accepts either a BIP39 mnemonic or a WIF private key in a single field, dispatching by whitespace presence. WIF flow looks up the identity via byPublicKeyHash, validates the matched key is an AUTHENTICATION key at HIGH or CRITICAL level (rejecting MASTER which can't sign documents), and rejects disabled keys. Failed logins no longer clobber prior session state — they restore the prior status, or fall back to browsing if a remembered identity exists.

Co-Authored-By: Claude Opus 4.7 (1M context)
Eagerly resolve the identity (and DPNS name) for a pasted WIF once it
passes a cheap structural gate (length 51/52, base58 charset) plus a
400ms debounce. Surface "wrong key type" / "key disabled" warnings
pre-submit so users see the issue before clicking Login. Splits a
signer-free `resolveIdentityFromWif` out of `loginWithPrivateKey` for
shared use; wrong-purpose errors now also carry the security-level name
so MASTER auth keys read distinctly from purpose mismatches.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

📝 Walkthrough

Walkthrough

Adds secret-based login supporting mnemonic or WIF private keys: input-shape detection, lightweight WIF gating, a WIF-to-identity resolution + one-key signer path, a debounced WIF preview hook, session wiring for private-key auth, UI adjustments in the login modal, and comprehensive tests for these flows.

Changes

WIF and Private Key Login

Layer / File(s) Summary
Type Definitions & Detection
example-apps/dashnote/src/dash/types.ts, example-apps/dashnote/src/lib/detectSecretShape.ts
Adds DashSdk.identities.byPublicKeyHash(publicKeyHash: Uint8Array); introduces SecretShape, detectSecretShape(input), and looksLikeWif(input) (structural gate: trimmed length 51–52, base58 charset).
Core WIF Resolution
example-apps/dashnote/src/dash/loginWithPrivateKey.ts
New module implementing WIF parsing/resolution: resolveIdentityFromWif(), loginWithPrivateKey(), helper extractors, and error classes (InvalidPrivateKeyError, UnknownIdentityError, WrongKeyPurposeError, KeyDisabledError); finds identity by public-key-hash and validates purpose/security level.
Key Manager Helper
example-apps/dashnote/src/session/keyManagerFromKey.ts
Adds keyManagerFromKey(identityId, auth) returning a minimal DashKeyManager that wraps a pre-resolved DashAuth.
WIF Preview Hook
example-apps/dashnote/src/hooks/useWifPreview.ts
New useWifPreview(sdk, secret, enabled) hook with debounce and module-scoped lazy-load cache; resolves WIF -> identity + DPNS name and returns WifPreviewState variants (idle/checking/resolved/wrong-purpose/key-disabled).
Session Integration
example-apps/dashnote/src/session/SessionContext.tsx
login signature changed to accept secret; detects shape and routes to mnemonic path (IdentityKeyManager.create) or WIF path (loginWithPrivateKey + keyManagerFromKey); restores previous session/remembered identity state on failures and logs errors.
UI Update
example-apps/dashnote/src/components/LoginModal.tsx
Replaces mnemonic-only input with single secret field labeled “Mnemonic or Private Key”; computes secretShape, uses useWifPreview for realtime feedback, hides identity-index for WIF inputs, shows informational note, and disables Login when secret empty or preview blocks login.
Tests
example-apps/dashnote/test/*
Adds/updates tests: detectSecretShape.test.ts, loginWithPrivateKey.test.ts, useWifPreview.test.tsx, LoginModal.test.tsx, SessionContext.test.tsx covering detection, WIF resolution success/failure, hook debounce/caching, UI branches, and session fallback behaviors.

Sequence Diagram

sequenceDiagram
  participant User as User
  participant LoginModal as LoginModal
  participant Preview as useWifPreview
  participant Session as SessionContext
  participant LoginPK as loginWithPrivateKey
  participant SDK as DashSdk
  participant DPNS as DPNS

  User->>LoginModal: Enter secret (mnemonic or WIF)
  LoginModal->>Preview: detectSecretShape(secret) & preview(secret)
  Preview-->>LoginModal: preview state (idle/checking/resolved/error)
  User->>LoginModal: Submit secret
  LoginModal->>Session: submit secret
  Session->>Session: detectSecretShape(secret)
  alt mnemonic
    Session->>Session: IdentityKeyManager.create(mnemonic)
    Session-->>LoginModal: success (keyManager)
  else wif
    Session->>LoginPK: loginWithPrivateKey(wif)
    LoginPK->>SDK: identities.byPublicKeyHash(pubKeyHash)
    SDK-->>LoginPK: identity (or null)
    LoginPK->>LoginPK: validate purpose/security, build auth
    LoginPK-->>Session: auth + identityId
  end
  Session->>DPNS: resolve name (optional)
  DPNS-->>Session: name
  Session-->>LoginModal: success / failure
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 A secret tucked beneath the hay,
Mnemonics and WIFs now share the way.
Preview hums, it peeks and stays,
One-key signs the sunny days—
The rabbit hops, delighted, gay.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.10% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(dashnote): preview identity + key-type fitness on WIF paste' accurately and specifically describes the main changes: adding WIF (private key) login support with identity preview and key-type validation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/login-by-key

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 and usage tips.

…equirement

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@thephez thephez marked this pull request as ready for review May 6, 2026 13:41
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
example-apps/dashnote/src/session/SessionContext.tsx (1)

74-74: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

SessionValue.login parameter still typed as mnemonic.

The implementation now accepts secret: string (Line 147) and dispatches via detectSecretShape, but the public type on the context still reads login: (mnemonic: string, options?: LoginOptions) => Promise<void>. Rename for accuracy so callers / docs reflect that this accepts either a mnemonic or a WIF.

♻️ Proposed fix
-  login: (mnemonic: string, options?: LoginOptions) => Promise<void>;
+  login: (secret: string, options?: LoginOptions) => Promise<void>;
🤖 Prompt for 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.

In `@example-apps/dashnote/src/session/SessionContext.tsx` at line 74, The public
SessionValue.login type is incorrect: it names the parameter "mnemonic" but the
implementation accepts a generic secret and calls detectSecretShape; update the
context signature for SessionValue.login to use secret instead of mnemonic (i.e.
login: (secret: string, options?: LoginOptions) => Promise<void>) so callers and
docs match the implementation that dispatches via detectSecretShape; make the
same parameter rename in any exported types/interfaces that reference
SessionValue.login to keep types consistent.
🧹 Nitpick comments (5)
example-apps/dashnote/src/dash/loginWithPrivateKey.ts (2)

84-117: 💤 Low value

Doc says "base64 first, then hex" but the code tries hex first.

The block comment claims base64 is tried first as the preferred path, but tryDecodeKeyData actually checks the hex regex first (Lines 95-105) and only falls through to base64 (Lines 107-117). Functionally this is fine because the hex regex is strict enough that base64-encoded keys with +///= won't collide, but the comment misleads readers about intent. Either swap the order to match the comment, or update the comment to reflect "hex first because the regex is more discriminating".

🤖 Prompt for 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.

In `@example-apps/dashnote/src/dash/loginWithPrivateKey.ts` around lines 84 - 117,
The function tryDecodeKeyData currently attempts hex decoding first then falls
back to base64, but the block comment claims base64 is tried first; fix by
either swapping the decode order in tryDecodeKeyData so base64 is attempted
before the hex-regex branch, or update the block comment to state that hex is
checked first (because of the strict hex-regex) and base64 is the
fallback—ensure the comment and the implementation for tryDecodeKeyData remain
consistent.

291-304: 💤 Low value

Hex fallback doesn't guard against odd-length output.

tryDecodeKeyData correctly checks data.length % 2 === 0 (Line 95) before parsing as hex, but the equivalent path here (Line 294) only validates the alphabet. If pubKey.toString("hex") ever returned an odd-length string, new Uint8Array(hex.length / 2) truncates and the trailing nibble is silently dropped, producing wrong bytes. The mismatch then surfaces as UnknownIdentityError rather than a parse failure, which is misleading to debug. Probability is low (the SDK encoder is deterministic), but the asymmetry with tryDecodeKeyData is easy to fix.

♻️ Proposed fix
   if (typeof candidate.toString === "function") {
     try {
       const hex = candidate.toString("hex");
-      if (typeof hex === "string" && /^[0-9a-fA-F]+$/.test(hex)) {
+      if (
+        typeof hex === "string" &&
+        hex.length % 2 === 0 &&
+        /^[0-9a-fA-F]+$/.test(hex)
+      ) {
         const bytes = new Uint8Array(hex.length / 2);
🤖 Prompt for 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.

In `@example-apps/dashnote/src/dash/loginWithPrivateKey.ts` around lines 291 -
304, The hex-path in loginWithPrivateKey.ts currently validates only the hex
alphabet but not length, so add the same even-length guard used in
tryDecodeKeyData: after computing const hex = candidate.toString("hex"), check
hex.length % 2 === 0 before allocating the Uint8Array and parsing; if the length
is odd, fall through or return a parse failure instead of silently truncating.
Locate the block quoting candidate.toString, the hex variable and the Uint8Array
creation to add this check.
example-apps/dashnote/src/hooks/useWifPreview.ts (1)

39-44: 💤 Low value

Module-scoped cache is unbounded — fine for short sessions, worth bounding for long-lived tabs.

previewCache is a Map<string, WifPreviewState> that's never evicted (only _resetWifPreviewCacheForTests clears it). Each unique pasted WIF accumulates an entry for the lifetime of the page. In practice users don't paste many WIFs, so the leak is small, but a simple bounded LRU (or just capping at e.g. 32 entries with a FIFO eviction) would prevent surprises in long-running tabs. Optional.

🤖 Prompt for 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.

In `@example-apps/dashnote/src/hooks/useWifPreview.ts` around lines 39 - 44, The
module-scoped previewCache (Map<string, WifPreviewState>) is unbounded and can
grow for long-lived tabs; change it to a bounded cache by implementing LRU or
simple FIFO eviction (suggest cap = 32). Specifically, replace or wrap
previewCache logic so that whenever an entry is inserted (wherever
previewCache.set(...) is called in useWifPreview.ts) you enforce the cap: if
size >= 32 evict the least-recent (LRU) or oldest key before inserting the new
entry; keep the _resetWifPreviewCacheForTests() helper to clear the cache for
tests and ensure WifPreviewState values are preserved during eviction
bookkeeping.
example-apps/dashnote/test/SessionContext.test.tsx (1)

34-40: 💤 Low value

Mocked error classes diverge slightly from the real shapes.

The real WrongKeyPurposeError carries identityId, purposeName, and securityLevelName, and KeyDisabledError carries identityId. The bare extends Error {} mocks here are fine for the current SessionContext branch (which appears to dispatch on message text), but if SessionContext ever starts reading those fields the tests will silently keep passing on undefined. Consider mirroring the real constructors so the mocks fail loudly if a future refactor depends on those fields.

🤖 Prompt for 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.

In `@example-apps/dashnote/test/SessionContext.test.tsx` around lines 34 - 40, The
mock error classes in the vi.mock for "../src/dash/loginWithPrivateKey" should
mirror the real error shapes so tests fail loudly if code reads their fields:
update the mock classes WrongKeyPurposeError and KeyDisabledError (and
optionally UnknownIdentityError/InvalidPrivateKeyError) to accept the same
constructor parameters and assign properties (e.g.,
WrongKeyPurposeError(identityId, purposeName, securityLevelName) and
KeyDisabledError(identityId)) onto the instance, and keep
mockLoginWithPrivateKey unchanged; locate these class definitions in the vi.mock
block and add matching constructors that set the fields used by SessionContext.
example-apps/dashnote/test/useWifPreview.test.tsx (1)

105-115: 💤 Low value

Document flushDebounce's coupling to internal microtask depth.

await Promise.resolve() twice happens to match the current resolver→DPNS chain. If the hook later inserts another await (e.g., adds an additional intermediate step before settling state), this helper will silently stop flushing far enough and the tests after it will see stale state. Worth either adding a brief note that the count must match the longest internal await chain, or replacing with a small loop until a quiescent state is observed.

🤖 Prompt for 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.

In `@example-apps/dashnote/test/useWifPreview.test.tsx` around lines 105 - 115,
The helper flushDebounce is tightly coupled to the hook's current internal
microtask depth because it uses two explicit await Promise.resolve() calls;
either add a clear comment above flushDebounce stating that the number of awaits
must match the longest internal await chain in the hook, or (preferably) replace
the two fixed awaits with a short loop that awaits Promise.resolve() repeatedly
until the component/test state is observed to be stable (break after a safe max
iterations like 10 to avoid infinite loops); reference the flushDebounce
function and the tests that call it when implementing this change so the loop
checks the same observable (DOM/state) the tests rely on to detect quiescence.
🤖 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 `@example-apps/dashnote/src/hooks/useWifPreview.ts`:
- Around line 1-11: The static imports KeyDisabledError, resolveIdentityFromWif,
UnknownIdentityError, and WrongKeyPurposeError from
"../dash/loginWithPrivateKey" (and resolveDpnsName if it triggers the SDK) pull
the ~8MB WASM SDK into the app shell; move those imports into the effect in
useWifPreview (the effect that runs when a user pastes a WIF) and replace them
with dynamic import() calls so the SDK is fetched only on first
use—specifically, inside the effect await import("../dash/loginWithPrivateKey")
to get
KeyDisabledError/resolveIdentityFromWif/UnknownIdentityError/WrongKeyPurposeError
(and await import("../dash/resolveDpnsName") if resolveDpnsName is used there),
then call resolveIdentityFromWif and handle the errors as before; keep
looksLikeWif as a static import since it’s small and used during render.

In `@example-apps/dashnote/src/session/SessionContext.tsx`:
- Around line 16-19: The static import of loginWithPrivateKey in SessionContext
pulls the large WASM SDK at module-evaluation time; instead remove the top-level
import and dynamically import the module inside the WIF branch of the login
function (the branch that handles wallet import format), e.g. use await
import("../dash/loginWithPrivateKey") within login so the heavy `@dashevo/evo-sdk`
bundle is only loaded on first auth; keep loadSdkModule() usage for other flows
and ensure types (DashKeyManager, DashSdk) remain via type imports only.

---

Outside diff comments:
In `@example-apps/dashnote/src/session/SessionContext.tsx`:
- Line 74: The public SessionValue.login type is incorrect: it names the
parameter "mnemonic" but the implementation accepts a generic secret and calls
detectSecretShape; update the context signature for SessionValue.login to use
secret instead of mnemonic (i.e. login: (secret: string, options?: LoginOptions)
=> Promise<void>) so callers and docs match the implementation that dispatches
via detectSecretShape; make the same parameter rename in any exported
types/interfaces that reference SessionValue.login to keep types consistent.

---

Nitpick comments:
In `@example-apps/dashnote/src/dash/loginWithPrivateKey.ts`:
- Around line 84-117: The function tryDecodeKeyData currently attempts hex
decoding first then falls back to base64, but the block comment claims base64 is
tried first; fix by either swapping the decode order in tryDecodeKeyData so
base64 is attempted before the hex-regex branch, or update the block comment to
state that hex is checked first (because of the strict hex-regex) and base64 is
the fallback—ensure the comment and the implementation for tryDecodeKeyData
remain consistent.
- Around line 291-304: The hex-path in loginWithPrivateKey.ts currently
validates only the hex alphabet but not length, so add the same even-length
guard used in tryDecodeKeyData: after computing const hex =
candidate.toString("hex"), check hex.length % 2 === 0 before allocating the
Uint8Array and parsing; if the length is odd, fall through or return a parse
failure instead of silently truncating. Locate the block quoting
candidate.toString, the hex variable and the Uint8Array creation to add this
check.

In `@example-apps/dashnote/src/hooks/useWifPreview.ts`:
- Around line 39-44: The module-scoped previewCache (Map<string,
WifPreviewState>) is unbounded and can grow for long-lived tabs; change it to a
bounded cache by implementing LRU or simple FIFO eviction (suggest cap = 32).
Specifically, replace or wrap previewCache logic so that whenever an entry is
inserted (wherever previewCache.set(...) is called in useWifPreview.ts) you
enforce the cap: if size >= 32 evict the least-recent (LRU) or oldest key before
inserting the new entry; keep the _resetWifPreviewCacheForTests() helper to
clear the cache for tests and ensure WifPreviewState values are preserved during
eviction bookkeeping.

In `@example-apps/dashnote/test/SessionContext.test.tsx`:
- Around line 34-40: The mock error classes in the vi.mock for
"../src/dash/loginWithPrivateKey" should mirror the real error shapes so tests
fail loudly if code reads their fields: update the mock classes
WrongKeyPurposeError and KeyDisabledError (and optionally
UnknownIdentityError/InvalidPrivateKeyError) to accept the same constructor
parameters and assign properties (e.g., WrongKeyPurposeError(identityId,
purposeName, securityLevelName) and KeyDisabledError(identityId)) onto the
instance, and keep mockLoginWithPrivateKey unchanged; locate these class
definitions in the vi.mock block and add matching constructors that set the
fields used by SessionContext.

In `@example-apps/dashnote/test/useWifPreview.test.tsx`:
- Around line 105-115: The helper flushDebounce is tightly coupled to the hook's
current internal microtask depth because it uses two explicit await
Promise.resolve() calls; either add a clear comment above flushDebounce stating
that the number of awaits must match the longest internal await chain in the
hook, or (preferably) replace the two fixed awaits with a short loop that awaits
Promise.resolve() repeatedly until the component/test state is observed to be
stable (break after a safe max iterations like 10 to avoid infinite loops);
reference the flushDebounce function and the tests that call it when
implementing this change so the loop checks the same observable (DOM/state) the
tests rely on to detect quiescence.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 05fe77bf-9740-4635-bff8-cc92336b0706

📥 Commits

Reviewing files that changed from the base of the PR and between fb19a9c and 883bdbe.

📒 Files selected for processing (12)
  • example-apps/dashnote/src/components/LoginModal.tsx
  • example-apps/dashnote/src/dash/loginWithPrivateKey.ts
  • example-apps/dashnote/src/dash/types.ts
  • example-apps/dashnote/src/hooks/useWifPreview.ts
  • example-apps/dashnote/src/lib/detectSecretShape.ts
  • example-apps/dashnote/src/session/SessionContext.tsx
  • example-apps/dashnote/src/session/keyManagerFromKey.ts
  • example-apps/dashnote/test/LoginModal.test.tsx
  • example-apps/dashnote/test/SessionContext.test.tsx
  • example-apps/dashnote/test/detectSecretShape.test.ts
  • example-apps/dashnote/test/loginWithPrivateKey.test.ts
  • example-apps/dashnote/test/useWifPreview.test.tsx

Comment thread example-apps/dashnote/src/hooks/useWifPreview.ts
Comment thread example-apps/dashnote/src/session/SessionContext.tsx Outdated
The eager-preview hook and SessionContext were statically importing
loginWithPrivateKey, which transitively pulls @dashevo/evo-sdk (~8MB
WASM) into the entry chunk and defeats the lazy-load strategy.
Switch both to dynamic imports gated behind the post-debounce timer
(hook) and the WIF login branch (context); rename SessionValue.login's
parameter mnemonic→secret to match the implementation since the prior
WIF-login PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@example-apps/dashnote/src/hooks/useWifPreview.ts`:
- Around line 42-45: The hook currently caches raw WIF strings (previewCache
keyed by the full WIF and Resolved.wif stored in state), which retains secrets
beyond the form lifecycle; change useWifPreview so previews are only kept
component-local and cleared whenever the input WIF changes or modal closes: stop
storing the raw WIF in previewCache and in Resolved.wif, instead cache by a
non-secret derivative (e.g., a hash/nonce) or remove caching entirely, keep only
the preview state (WifPreviewState) tied to the current input, and ensure any
existing previewCache entries are cleared on input change or unmount in the hook
(reference previewCache, Resolved, resolved.wif, and useWifPreview to locate and
fix).
- Around line 91-149: The lazy-import (loadLoginModule) is awaited outside the
try, so import/chunk fetch failures escape and leave the preview stuck in
"checking"; move the await of loadLoginModule() into the existing try block (or
wrap it in its own try/catch) so any errors from loadLoginModule are caught and
collapse to the same idle/silent outcome (set next = IDLE, avoid caching, and
return via the same cancelled checks), keeping references to loadLoginModule,
mod.resolveIdentityFromWif, previewCache, and setResolved to locate the change.

In `@example-apps/dashnote/src/session/SessionContext.tsx`:
- Around line 216-229: The catch path currently prefers loadRememberedIdentity()
and overwrites the live session snapshot, causing an authenticated user to be
forced into "browsing"; change the logic in SessionContext so you only apply the
remembered identity when there is no valid prior session: check priorKeyManager
(and/or priorStatus not indicating an authenticated session) before calling
setKeyManager/setIdentityId/setDpnsName/setStatus with the remembered values;
otherwise restore the prior snapshot by assigning priorKeyManager,
priorIdentityId, priorDpnsName, and priorStatus back to the state variables
instead of switching to "browsing" (use the existing symbols
loadRememberedIdentity, priorKeyManager, priorIdentityId, priorDpnsName,
priorStatus, setKeyManager, setIdentityId, setDpnsName, setStatus).
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d553c2d-04b7-4adf-a04c-70fc44860da2

📥 Commits

Reviewing files that changed from the base of the PR and between 883bdbe and 2b2c5a0.

📒 Files selected for processing (2)
  • example-apps/dashnote/src/hooks/useWifPreview.ts
  • example-apps/dashnote/src/session/SessionContext.tsx

Comment thread example-apps/dashnote/src/hooks/useWifPreview.ts Outdated
Comment thread example-apps/dashnote/src/hooks/useWifPreview.ts
Comment thread example-apps/dashnote/src/session/SessionContext.tsx Outdated
- Drop the module-scoped previewCache in useWifPreview: cached raw WIFs
  survived modal close/reopen for the page lifetime, extending secret
  retention beyond the form. Resolution state is now strictly
  component-local.
- Move the loadLoginModule() await inside the try in useWifPreview so a
  chunk-fetch failure collapses to idle instead of stranding the
  preview on "checking".
- Restore the active session in SessionContext's login catch path when
  priorStatus is "authenticated": typing a bad secret while signed in
  no longer demotes the user to browsing, even when a remembered
  identity is on disk. Switch flows already logout() before login()
  and are unaffected.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@thephez
Copy link
Copy Markdown
Collaborator Author

thephez commented May 6, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
example-apps/dashnote/src/session/SessionContext.tsx (1)

188-189: 💤 Low value

Redundant nullish-coalesce.

resolvedId is already ... ?? null, so the second ?? null on line 189 is a no-op.

♻️ Proposed tweak
         const resolvedId = resolvedKeyManager.identityId ?? null;
-        setIdentityId(resolvedId ?? null);
+        setIdentityId(resolvedId);
🤖 Prompt for 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.

In `@example-apps/dashnote/src/session/SessionContext.tsx` around lines 188 - 189,
The code creates resolvedId using resolvedKeyManager.identityId ?? null and then
calls setIdentityId(resolvedId ?? null), which is redundant; change the
setIdentityId call to pass resolvedId directly (or inline the expression once)
so you remove the duplicate "?? null" — update the usage in SessionContext
(resolvedId / setIdentityId) accordingly.
example-apps/dashnote/test/useWifPreview.test.tsx (1)

1-327: 💤 Low value

LGTM — thorough state-by-state coverage with proper fake-timer discipline.

The use of real-shaped error classes via vi.hoisted (rather than bare extends Error) is the right call since the hook relies on instanceof mod.WrongKeyPurposeError / mod.KeyDisabledError checks. The debounce-cancellation, whitespace-trim, and cross-mount-no-cache tests are particularly valuable for locking in the secret-retention guarantees the hook is responsible for.

One coverage gap worth considering (optional): the hook explicitly handles loadLoginModule() rejection (chunk fetch failure / offline) by falling through to IDLE via the mod && ... guard at lines 103-104. There's no test that exercises that branch, so a future refactor that hoists loadLoginModule() back outside the try (the original bug) wouldn't be caught here. A small test that mocks the dynamic import to reject could lock that in, but it requires vi.mock with a rejecting factory or module-cache manipulation, so it may not be worth the friction.

🤖 Prompt for 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.

In `@example-apps/dashnote/test/useWifPreview.test.tsx` around lines 1 - 327, Add
a test that exercises the hook path where the dynamic import/loadLoginModule
rejects so the hook falls back to idle; specifically, mock the module load to
throw (simulate chunk fetch/offline) and then render Harness and flushDebounce,
asserting the final state is "idle" and that mockResolveIdentityFromWif and
mockResolveDpnsName were not called. Target the useWifPreview behavior and the
dynamic import/loader (loadLoginModule) branch — ensure the mock makes the
importer reject before the hook attempts resolution so the guard that checks the
loaded module is exercised.
🤖 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.

Nitpick comments:
In `@example-apps/dashnote/src/session/SessionContext.tsx`:
- Around line 188-189: The code creates resolvedId using
resolvedKeyManager.identityId ?? null and then calls setIdentityId(resolvedId ??
null), which is redundant; change the setIdentityId call to pass resolvedId
directly (or inline the expression once) so you remove the duplicate "?? null" —
update the usage in SessionContext (resolvedId / setIdentityId) accordingly.

In `@example-apps/dashnote/test/useWifPreview.test.tsx`:
- Around line 1-327: Add a test that exercises the hook path where the dynamic
import/loadLoginModule rejects so the hook falls back to idle; specifically,
mock the module load to throw (simulate chunk fetch/offline) and then render
Harness and flushDebounce, asserting the final state is "idle" and that
mockResolveIdentityFromWif and mockResolveDpnsName were not called. Target the
useWifPreview behavior and the dynamic import/loader (loadLoginModule) branch —
ensure the mock makes the importer reject before the hook attempts resolution so
the guard that checks the loaded module is exercised.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f3b7a23b-5210-4038-babd-f152ffd2cb14

📥 Commits

Reviewing files that changed from the base of the PR and between 2b2c5a0 and ed4245e.

📒 Files selected for processing (4)
  • example-apps/dashnote/src/hooks/useWifPreview.ts
  • example-apps/dashnote/src/session/SessionContext.tsx
  • example-apps/dashnote/test/SessionContext.test.tsx
  • example-apps/dashnote/test/useWifPreview.test.tsx

@thephez thephez merged commit e251265 into main May 6, 2026
3 checks passed
@thephez thephez deleted the feat/login-by-key branch May 6, 2026 16:24
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