fix(shared,client): harden passkey recovery#556
Conversation
Adds Pimlico passkey-server registration and recovery behind a default-off flag, preserves local fallback metadata, and updates guarded recovery UI for PRD-537 and PRD-538.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an optional Pimlico-hosted passkey-server recovery flow (gated by VITE_PASSKEY_SERVER_ENABLED=false by default), server-first registration/authentication with local-storage fallbacks, expected smart-account address persistence, progressive recovery UI/state, unified passkey telemetry, tests, i18n, docs, and plan/status artifacts. ChangesHosted Passkey Server Recovery Flow
Sequence Diagram(s) sequenceDiagram
participant LoginView
participant AuthServices
participant PimlicoPasskeyServer
participant WebAuthn
participant SessionStorage
LoginView->>AuthServices: handlePasskeyLogin(userName)
AuthServices->>PimlicoPasskeyServer: startAuthentication(recoveryContext)
PimlicoPasskeyServer-->>AuthServices: credential + challenge or empty
AuthServices->>WebAuthn: perform assertion (challenge, credentialId)
WebAuthn-->>AuthServices: assertion response
AuthServices->>PimlicoPasskeyServer: verifyAuthentication(response)
AuthServices->>SessionStorage: setStoredSmartAccountAddress(address)
AuthServices-->>LoginView: emit session or fallback/error
Estimated code review effort: Possibly related PRs:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Oba-One
added
the
automated/codex
ChatGPT Codex Connector
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/shared/src/workflows/authServices.ts (1)
393-401: 💤 Low valueUnsafe cast of WebAuthn response to P256Credential["raw"].
The
authResponsefromnavigator.credentials.get()is aCredentialRequestOptionsresult (aPublicKeyCredential), but you're casting it directly toP256Credential["raw"]on line 399. This works in the happy path becausePublicKeyCredentialis structurally compatible, but the cast bypasses type checking. The earlier server registration flow (line 350) correctly passescreatedCredential.rawbecausecreateWebAuthnCredentialreturns a properly typed object.Consider extracting the raw credential consistently or documenting why this cast is safe for server authentication.
🤖 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 `@packages/shared/src/workflows/authServices.ts` around lines 393 - 401, The code unsafely casts authResponse to P256Credential["raw"]; instead, convert the PublicKeyCredential returned from navigator.credentials.get() into the same typed shape used during registration (use the helper that produced createdCredential.raw) and pass that typed raw to toVerifiedCredential; specifically, replace the direct cast of authResponse with a call that builds a P256Credential from the PublicKeyCredential (e.g., use the existing createWebAuthnCredential/creation helper or a small extractor that takes authResponse (PublicKeyCredential) and returns its typed .raw) and then call toVerifiedCredential(verification, typedRaw, "Passkey server authentication failed") to avoid bypassing type checks.
🤖 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 `@packages/client/src/views/Login/index.tsx`:
- Around line 68-74: The error branch in Login/index.tsx that checks the
variable msg (the block using msg.includes("expected account") ||
msg.includes("address")) is too broad and should be tightened to only match the
exact backend recovery phrases so it doesn't override more specific states;
update the condition(s) to test for explicit full phrases (e.g., the exact
recovery messages returned by the backend such as "expected account address" or
"address unavailable for recovery") instead of single words like "address" or
"unavailable", and apply the same tightening to the similar branch around lines
89-95 so the intl.formatMessage call for id "app.login.error.addressMismatch"
only runs for those explicit messages.
- Around line 587-615: The tertiaryAction currently exposes
handleStartSeparateAccount in the fallback branch, allowing users to create a
separate account before recovery is attempted; change the tertiaryAction logic
so that browserGuidanceTertiaryAction is returned if present, otherwise only
show the "connect wallet" action when recoveryAttempted && loginError, and do
NOT render the create-separate-account action in the tertiary fallback — instead
return walletAction or null/undefined; update the tertiaryAction conditional
(the expression referencing browserGuidanceTertiaryAction, recoveryAttempted,
loginError, handleWalletLogin, and handleStartSeparateAccount) so
createSeparateAccount is only ever shown when recoveryAttempted && loginError
(as already done in secondaryAction).
In `@packages/shared/src/config/passkeyServer.ts`:
- Around line 159-161: Normalize the VITE_PASSKEY_RP_ID value before returning
to avoid rp_origin_mismatch by trimming whitespace and lowercasing the host;
replace the direct return of envRpId (from env.VITE_PASSKEY_RP_ID) with a
normalized form such as envRpId.trim().toLowerCase() in the function that reads
envRpId so callers (the RP ID logic around rp_origin_mismatch) always get a
normalized hostname.
In `@packages/shared/src/modules/auth/session.ts`:
- Around line 105-111: Change the smart-account and embedded address storage
helpers to use viem's Address type instead of plain string: import Address from
'viem' (or import type { Address } from 'viem') and update the signatures of
setStoredSmartAccountAddress and getStoredSmartAccountAddress (and
setEmbeddedAddress/getEmbeddedAddress) so setters accept Address and getters
return Address | null; keep the same localStorage keys and JSON/string handling
but ensure the returned value is typed as Address | null and any callers are
updated to accept the stronger type.
---
Nitpick comments:
In `@packages/shared/src/workflows/authServices.ts`:
- Around line 393-401: The code unsafely casts authResponse to
P256Credential["raw"]; instead, convert the PublicKeyCredential returned from
navigator.credentials.get() into the same typed shape used during registration
(use the helper that produced createdCredential.raw) and pass that typed raw to
toVerifiedCredential; specifically, replace the direct cast of authResponse with
a call that builds a P256Credential from the PublicKeyCredential (e.g., use the
existing createWebAuthnCredential/creation helper or a small extractor that
takes authResponse (PublicKeyCredential) and returns its typed .raw) and then
call toVerifiedCredential(verification, typedRaw, "Passkey server authentication
failed") to avoid bypassing type checks.
🪄 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: b68cd4bf-0f2c-4e9a-9f87-350445d46449
📒 Files selected for processing (27)
.env.template.plans/active/account-recovery-hardening/handoffs/codex-state-api.md.plans/active/account-recovery-hardening/plan.todo.md.plans/active/account-recovery-hardening/status.jsondocs/docs/builders/architecture/sequence-diagrams.mdxdocs/docs/builders/journeys/onboarding.mdxdocs/docs/builders/packages/client-pwa-token-audit.generated.mdpackages/client/src/__tests__/views/AccountInfo.test.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/config/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/es.jsonpackages/shared/src/i18n/pt.jsonpackages/shared/src/index.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/workflows/authServices.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test
- GitHub Check: Test
- GitHub Check: Build Docs
- GitHub Check: Test
- GitHub Check: Playwright Client CI
- GitHub Check: CI Gate
- GitHub Check: Storybook
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (35)
packages/shared/**
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
@green-goods/sharedis the only home for reusable hooks, providers, stores, modules, types, i18n, and shared UI primitives
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/i18n/es.jsonpackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/i18n/en.jsonpackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/workflows/authServices.tspackages/shared/src/i18n/pt.json
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
Use centralized queryKeys, event-driven invalidation, useCurrentChain() or DEFAULT_CHAIN_ID, logger, and typed domain models such as Address in
@green-goods/sharedKeep reusable hooks, providers, stores, modules, and shared UI primitives in
@green-goods/shared
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/workflows/authServices.ts
packages/shared/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
Shared UI primitive changes should ship with tests, barrel updates, and Storybook coverage in the same change when applicable
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/workflows/authServices.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Default to single-chain behavior throughgetDefaultChain()orDEFAULT_CHAIN_ID
Useloggerfrom shared instead ofconsole.log
Usebun run format:check && bun lintfor code quality checks
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the
Addresstype for Ethereum addresses instead of raw string types
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
**/*.{json,ts,tsx}?(locales|i18n|translations|lang)
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add every new user-facing string to
en,es, andptlanguage files
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/i18n/es.jsonpackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/i18n/en.jsonpackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsxpackages/shared/src/i18n/pt.json
packages/shared/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/shared/AGENTS.md)
packages/shared/**/*.{ts,tsx}: Use centralized query keys fromqueryKeysinpackages/shared. Do not invent ad-hoc query arrays.
UseuseCurrentChain()orDEFAULT_CHAIN_IDfor application defaults inpackages/shared, not wallet chain state.
Prefer event-driven invalidation over polling inpackages/sharedhooks and queries.
Useloggerand typed domain models (Address, discriminated unions,unknownfor untrusted data) inpackages/sharedcode.
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/workflows/authServices.ts
packages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
Addresstype (notstring) for Ethereum addresses in TypeScript across all packages
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/workflows/authServices.ts
packages/{shared,client,admin}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never swallow errors — use
parseContractError()+USER_FRIENDLY_ERRORSfor contract errors, usecreateMutationErrorHandler()in shared mutation hooks, useloggerfrom shared (notconsole.log)
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx}: UsequeryKeys.*helpers from shared for React Query key serialization — serialize objects in query keys
Do not use banned vocabulary in i18n strings (enforced bybun run lint:vocab): never usestreak,countdown,leaderboard,FOMO, or growth-hacking language (urgent,limited time,re-engagement,retention hook)
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{contracts,agent,admin,shared}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use the
Addresstype for Ethereum addresses.
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/workflows/authServices.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
loggerfrom shared, neverconsole.log.
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin,shared}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Remixicon (
Ri*Line), never lucide.
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin,shared}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Any new user-facing string must be added to
en,es, andpt.
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin,shared}/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{client,admin,shared}/src/**/*.{ts,tsx,css}: TreatBaseline Widely Availableas the baseline target for frontend changes. Use repo-installed Modern Web Guidance throughbun run agentic:guidancebefore frontend, UI, CSS, accessibility, browser proof, or web-design changes.
Prefer semantic HTML, native controls, platform CSS, and browser primitives before custom JavaScript.
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin,shared}/src/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Banned vocabulary for any surface:
streak,countdown,leaderboard,FOMO.
Files:
packages/shared/src/config/index.tspackages/shared/src/__tests__/modules/session.test.tspackages/shared/src/index.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/utils/errors/contract-errors.tspackages/shared/src/i18n/es.jsonpackages/shared/src/modules/app/analytics-events.tspackages/shared/src/providers/Auth.tsxpackages/shared/src/modules/auth/session.tspackages/shared/src/i18n/en.jsonpackages/shared/src/modules/index.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/workflows/authServices.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsxpackages/shared/src/i18n/pt.json
{.github/workflows/**,**/*.test.{js,ts,jsx,tsx},**/*.spec.{js,ts,jsx,tsx}}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
bun run test, neverbun test
Files:
packages/shared/src/__tests__/modules/session.test.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
bun run testfor running tests
Files:
packages/shared/src/__tests__/modules/session.test.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bun run testinstead ofbun testfor running tests —bun testuses bun's built-in runner (ignores vitest config), whilebun run testruns the package.json script with proper vitest environment
Files:
packages/shared/src/__tests__/modules/session.test.tspackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/utils/errors/__tests__/contract-errors.test.tspackages/shared/src/__tests__/workflows/authServices.test.tspackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/shared/src/index.ts
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
Export public APIs through package barrels; do not normalize deep imports as the public contract in
@green-goods/shared
Files:
packages/shared/src/index.ts
packages/shared/src/i18n/{en,es,pt}.json
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
New user-facing strings must be added to src/i18n/en.json, src/i18n/es.json, and src/i18n/pt.json
Files:
packages/shared/src/i18n/es.jsonpackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/pt.json
packages/shared/src/i18n/*.json
📄 CodeRabbit inference engine (packages/shared/AGENTS.md)
Add every new user-facing string to
src/i18n/en.json,src/i18n/es.json, andsrc/i18n/pt.jsonin the shared package. Do not hardcode user-facing strings.
Files:
packages/shared/src/i18n/es.jsonpackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/pt.json
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Remixicon (
Ri*Line), never lucide for UI icons
Files:
packages/shared/src/providers/Auth.tsxpackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/shared/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Tailwind v4 does not scan
packages/shared/src/from admin/client builds due to content scanning limitations — do NOT add utility classes (mx-4,w-max,self-center,justify-self-center) directly to JSX in shared components; instead use inline styles for layout or CSS overrides in the consuming package
Files:
packages/shared/src/providers/Auth.tsx
packages/{client,admin,shared}/src/**/*.{tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Design System: Load
designskill (direction) +uiskill (implementation) explicitly when paradigm, layout composition, new view, tokens, or PR review is at stake — for trivial edits (padding, copy, single component touch), use the rules in CLAUDE.md
Files:
packages/shared/src/providers/Auth.tsxpackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin,shared}/src/**/*.{css,scss,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Design tokens: never hardcode
cubic-bezier,duration, or raw color/radius values — use--spring-*(6 motion tokens),--color-*,--radius-*,--color-material-*+--blur-material-*from theme.css
Files:
packages/shared/src/providers/Auth.tsxpackages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
.env*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
.env*: Use the root.envonly; do not add package-level.envfiles
Keep.env*files human-governed even when Copilot review runs automatically
Files:
.env.template
packages/client/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/instructions/client.instructions.md)
packages/client/**/*.{ts,tsx,js,jsx}: Hooks, providers, and most business logic should come from@green-goods/sharedin packages/client
Do not add local hooks or providers when the logic belongs in shared
Preserve the offline-first queue flow for work submission; do not bypass the queue for passkey users
Keep authentication branches on shared auth APIs and shared default-chain helpers instead of wallet chain state
Manage blob URLs through shared utilities such asmediaResourceManager; do not leave orphanedURL.createObjectURLvalues behind
Prefer event-driven invalidation over polling
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/client/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/client/AGENTS.md)
packages/client/**/*.{ts,tsx}: Do not create local hooks or providers in the client package when the logic belongs in@green-goods/shared
Work submission must preserve the offline-first queue flow; do not bypass the queue for passkey users
Prefer event-driven invalidation over polling
Manage blob URLs through shared utilities such asmediaResourceManager; do not leave orphanedURL.createObjectURLvalues behind
Authentication branches must go through shared auth APIs; do not treat wallet chain state as the source of truth for app defaults
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/client/AGENTS.md)
New user-facing strings must be translated in all three locale files
Client-only banned vocabulary (never use in client PWA):
operator cockpit,utility copy,KPI tile,dashboard,Plus Jakarta SansClient only: do not use
operator cockpit,utility copy,Plus Jakarta Sans,KPI tile,dashboard.
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/{client,admin}/src/**/*.{ts,tsx}: ALL React hooks MUST live in@green-goods/shared— client and admin packages must not define their own hooks and should import hooks from the shared package
Import hooks from@green-goods/sharedusing barrel imports:import { useAuth, useGardens } from '@green-goods/shared'— never use deep paths like@green-goods/shared/src/hooks/useAuth
Contract integration: Import deployment artifacts from../../../contracts/deployments/11155111-latest.json(or appropriate chain ID), never hardcode Ethereum addresses in code
UseisGreenWillDeployed(chainId?)from@green-goods/sharedto detect when a feature contract is undeployed (zero-address) on the active chain — render a 'not available on this network' branch instead of a generic empty state
When a useMemo depends on a value written to localStorage in the same tab (e.g., pending-join membership), includeusePendingJoinsVersion()from@green-goods/sharedin its useMemo deps — this returns an incrementing counter that ticks on every in-tab pending-join change
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin}/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
When debugging user-observed UI regressions, start from the rendered surface before tracing data flow — inspect DOM geometry, computed styles (bounding rect, width/height, opacity, display, pointer-events, z-index, overflow, disabled state, selected classes, border/ring), verify click/tap changes state, then trace visible element → component → state setter
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/client/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Client PWA uses
@green-goods/sharedprimitives + presentation-mode loaders/PublicShell/PwaRuntime/AppShell/SiteHeader/AppBar— do not invent component names; flag missing primitives instead
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{client,admin}/src/**/*.{tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Volume hierarchy: canvas 80–90% / ink 8–15% / stone 3–5% / accent green 1–3% — flooding the screen with green is the
#1design failure mode
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
packages/{contracts,agent,client,admin}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Default to single-chain behavior through
getDefaultChain()orDEFAULT_CHAIN_ID.
Files:
packages/client/src/components/Layout/Splash.tsxpackages/client/src/views/Login/index.tsxpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/__tests__/views/AccountInfo.test.tsx
🪛 ast-grep (0.43.0)
packages/shared/src/modules/auth/session.ts
[warning] 105-105: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(SMART_ACCOUNT_ADDRESS_STORAGE_KEY, address)
Note: [CWE-312] Cleartext Storage of Sensitive Information
(local-storage-sensitive-information)
🪛 LanguageTool
.plans/active/account-recovery-hardening/handoffs/codex-state-api.md
[grammar] ~40-~40: Use a hyphen to join words.
Context: ... ### Implemented - Added shared Pimlico hosted passkey-server helpers in `packag...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (31)
packages/shared/src/modules/app/analytics-events.ts (1)
24-36: LGTM!Also applies to: 139-145, 147-169, 182-184
packages/shared/src/workflows/authServices.ts (4)
118-142: LGTM!
160-215: LGTM!Also applies to: 240-252
326-356: LGTM!Also applies to: 358-405, 407-449
463-515: LGTM!Also applies to: 529-576, 590-666
packages/shared/src/utils/errors/contract-errors.ts (1)
471-491: LGTM!packages/shared/src/utils/errors/__tests__/contract-errors.test.ts (1)
193-199: LGTM!packages/shared/src/__tests__/workflows/authServices.test.ts (4)
119-134: LGTM!Also applies to: 142-171, 177-186
200-217: LGTM!Also applies to: 293-303
481-572: LGTM!
660-858: LGTM!packages/client/src/__tests__/views/Login.test.tsx (1)
24-34: LGTM!Also applies to: 43-45, 69-69, 93-94, 103-104, 114-114, 163-188, 196-202, 214-248, 255-265, 275-300, 305-306, 309-369, 378-380, 421-422
packages/client/src/__tests__/views/AccountInfo.test.tsx (1)
82-86: LGTM!Also applies to: 93-93, 101-101, 113-113, 119-119
docs/docs/builders/packages/client-pwa-token-audit.generated.md (1)
94-99: LGTM!docs/docs/builders/architecture/sequence-diagrams.mdx (1)
221-221: LGTM!Also applies to: 227-227, 234-242, 245-245, 261-271, 281-283
docs/docs/builders/journeys/onboarding.mdx (1)
72-72: LGTM!Also applies to: 82-85, 100-103
packages/client/src/components/Layout/Splash.tsx (1)
26-31: LGTM!Also applies to: 74-83, 89-103, 111-176, 186-187, 257-277
packages/shared/src/i18n/en.json (1)
1848-1850: LGTM!Also applies to: 1875-1894, 1902-1902, 1905-1913, 1922-1924
packages/shared/src/i18n/es.json (1)
1848-1850: LGTM!Also applies to: 1875-1890, 1902-1902, 1905-1913, 1922-1924
.plans/active/account-recovery-hardening/handoffs/codex-state-api.md (1)
36-99: LGTM!.plans/active/account-recovery-hardening/plan.todo.md (1)
35-43: LGTM!Also applies to: 113-130, 154-158
.plans/active/account-recovery-hardening/status.json (1)
13-13: LGTM!Also applies to: 82-113, 223-255
.env.template (1)
89-93: LGTM!packages/shared/src/config/passkeyServer.ts (1)
10-147: LGTM!Also applies to: 154-158, 164-175
packages/shared/src/config/index.ts (1)
57-72: LGTM!packages/shared/src/index.ts (1)
225-237: LGTM!packages/shared/src/__tests__/config/passkeyServer.test.ts (1)
1-130: LGTM!packages/shared/src/modules/auth/session.ts (1)
38-40: LGTM!Also applies to: 157-157, 180-203
packages/shared/src/modules/index.ts (1)
130-130: LGTM!Also applies to: 132-132, 137-137, 139-139
packages/shared/src/providers/Auth.tsx (1)
50-51: LGTM!Also applies to: 444-448, 475-479
packages/shared/src/__tests__/modules/session.test.ts (1)
44-54: LGTM!Also applies to: 126-148, 154-161
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/shared/src/modules/auth/session.ts (1)
111-112: 💤 Low valueConsider adding runtime validation for the type assertion.
The type assertion
as Address | nullis safe when inputs are controlled, but localStorage can be manually edited or corrupted. Adding a lightweight hex-string validation check would make this more defensive.♻️ Optional defensive validation
export function getStoredSmartAccountAddress(): Address | null { - return localStorage.getItem(SMART_ACCOUNT_ADDRESS_STORAGE_KEY) as Address | null; + const stored = localStorage.getItem(SMART_ACCOUNT_ADDRESS_STORAGE_KEY); + if (!stored) return null; + // Lightweight validation: check if it looks like a hex address + if (!/^0x[0-9a-fA-F]{40}$/.test(stored)) { + logger.warn("[Session] Invalid smart-account address in storage, clearing..."); + localStorage.removeItem(SMART_ACCOUNT_ADDRESS_STORAGE_KEY); + return null; + } + return stored as Address; }This matches the defensive pattern used for credential parsing at lines 254-266.
🤖 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 `@packages/shared/src/modules/auth/session.ts` around lines 111 - 112, getStoredSmartAccountAddress currently casts localStorage.getItem(SMART_ACCOUNT_ADDRESS_STORAGE_KEY) to Address | null without validating user-editable storage; add a lightweight runtime validation (e.g., ensure value is a string, matches a hex-address pattern like /^0x[0-9a-fA-F]{40}$/ or other project-specific Address format) and return null when invalid so the function only returns a valid Address or null; update getStoredSmartAccountAddress to perform this check before returning the value.
🤖 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 `@packages/shared/src/modules/auth/session.ts`:
- Around line 111-112: getStoredSmartAccountAddress currently casts
localStorage.getItem(SMART_ACCOUNT_ADDRESS_STORAGE_KEY) to Address | null
without validating user-editable storage; add a lightweight runtime validation
(e.g., ensure value is a string, matches a hex-address pattern like
/^0x[0-9a-fA-F]{40}$/ or other project-specific Address format) and return null
when invalid so the function only returns a valid Address or null; update
getStoredSmartAccountAddress to perform this check before returning the value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d127f57-03e9-4ca7-8942-8f40ccbe3cf1
📒 Files selected for processing (5)
packages/client/src/__tests__/views/Login.test.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/__tests__/config/passkeyServer.test.tspackages/shared/src/config/passkeyServer.tspackages/shared/src/modules/auth/session.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/shared/src/tests/config/passkeyServer.test.ts
- packages/shared/src/config/passkeyServer.ts
- packages/client/src/tests/views/Login.test.tsx
- packages/client/src/views/Login/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Test
- GitHub Check: CI Gate
- GitHub Check: Storybook
- GitHub Check: Playwright Admin CI
- GitHub Check: Test
- GitHub Check: Test
- GitHub Check: Build Docs
- GitHub Check: Playwright Client CI
- GitHub Check: Lint And Build
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (16)
packages/shared/**
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
@green-goods/sharedis the only home for reusable hooks, providers, stores, modules, types, i18n, and shared UI primitives
Files:
packages/shared/src/modules/auth/session.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
Use centralized queryKeys, event-driven invalidation, useCurrentChain() or DEFAULT_CHAIN_ID, logger, and typed domain models such as Address in
@green-goods/sharedKeep reusable hooks, providers, stores, modules, and shared UI primitives in
@green-goods/shared
Files:
packages/shared/src/modules/auth/session.ts
packages/shared/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
Shared UI primitive changes should ship with tests, barrel updates, and Storybook coverage in the same change when applicable
Files:
packages/shared/src/modules/auth/session.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Default to single-chain behavior throughgetDefaultChain()orDEFAULT_CHAIN_ID
Useloggerfrom shared instead ofconsole.log
Usebun run format:check && bun lintfor code quality checks
Files:
packages/shared/src/modules/auth/session.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the
Addresstype for Ethereum addresses instead of raw string types
Files:
packages/shared/src/modules/auth/session.ts
**/*.{json,ts,tsx}?(locales|i18n|translations|lang)
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add every new user-facing string to
en,es, andptlanguage files
Files:
packages/shared/src/modules/auth/session.ts
packages/shared/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/shared/AGENTS.md)
packages/shared/**/*.{ts,tsx}: Use centralized query keys fromqueryKeysinpackages/shared. Do not invent ad-hoc query arrays.
UseuseCurrentChain()orDEFAULT_CHAIN_IDfor application defaults inpackages/shared, not wallet chain state.
Prefer event-driven invalidation over polling inpackages/sharedhooks and queries.
Useloggerand typed domain models (Address, discriminated unions,unknownfor untrusted data) inpackages/sharedcode.
Files:
packages/shared/src/modules/auth/session.ts
packages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
Addresstype (notstring) for Ethereum addresses in TypeScript across all packages
Files:
packages/shared/src/modules/auth/session.ts
packages/{shared,client,admin}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never swallow errors — use
parseContractError()+USER_FRIENDLY_ERRORSfor contract errors, usecreateMutationErrorHandler()in shared mutation hooks, useloggerfrom shared (notconsole.log)
Files:
packages/shared/src/modules/auth/session.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx}: UsequeryKeys.*helpers from shared for React Query key serialization — serialize objects in query keys
Do not use banned vocabulary in i18n strings (enforced bybun run lint:vocab): never usestreak,countdown,leaderboard,FOMO, or growth-hacking language (urgent,limited time,re-engagement,retention hook)
Files:
packages/shared/src/modules/auth/session.ts
packages/{contracts,agent,admin,shared}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use the
Addresstype for Ethereum addresses.
Files:
packages/shared/src/modules/auth/session.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
loggerfrom shared, neverconsole.log.
Files:
packages/shared/src/modules/auth/session.ts
packages/{client,admin,shared}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Remixicon (
Ri*Line), never lucide.
Files:
packages/shared/src/modules/auth/session.ts
packages/{client,admin,shared}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Any new user-facing string must be added to
en,es, andpt.
Files:
packages/shared/src/modules/auth/session.ts
packages/{client,admin,shared}/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{client,admin,shared}/src/**/*.{ts,tsx,css}: TreatBaseline Widely Availableas the baseline target for frontend changes. Use repo-installed Modern Web Guidance throughbun run agentic:guidancebefore frontend, UI, CSS, accessibility, browser proof, or web-design changes.
Prefer semantic HTML, native controls, platform CSS, and browser primitives before custom JavaScript.
Files:
packages/shared/src/modules/auth/session.ts
packages/{client,admin,shared}/src/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Banned vocabulary for any surface:
streak,countdown,leaderboard,FOMO.
Files:
packages/shared/src/modules/auth/session.ts
🪛 ast-grep (0.43.0)
packages/shared/src/modules/auth/session.ts
[warning] 106-106: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(SMART_ACCOUNT_ADDRESS_STORAGE_KEY, address)
Note: [CWE-312] Cleartext Storage of Sensitive Information
(local-storage-sensitive-information)
[warning] 128-128: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(EMBEDDED_ADDRESS_KEY, address)
Note: [CWE-312] Cleartext Storage of Sensitive Information
(local-storage-sensitive-information)
🔇 Additional comments (6)
packages/shared/src/modules/auth/session.ts (6)
17-17: LGTM!
39-40: LGTM!
105-118: LGTM!
128-134: LGTM!
158-158: LGTM!
203-203: LGTM!
…tion, and restore Production-readiness review fixes for the account-recovery-hardening lane: - Tag hosted passkey-server lookup failures by call phase (PasskeyServerLookupError) so viem HttpRequestError/TimeoutError shapes reach the legacy local fallback during outages; classify WebAuthn cancellations and transport failures by error name, not message text. - Skip the expected-address continuity assert during registration: a confirmed separate account is a new address by design, and stale expected-address state must not dead-end account creation or burn a recovery name server-side. - Fail closed in session restore when the stored credential rebuilds a different address than expected (reason-coded telemetry, nothing cleared). - Stop fabricating a "user" placeholder username in one-tap login and switch-to-passkey; empty routes straight to the local path instead of a hosted-server lookup for a literal placeholder name. - Add a guarded "Recover with username" entry for returning users that never exposes separate-account creation, with en/es/pt strings. - Narrow telemetry address_mismatch matching; add QA notes for RP-blocked tunnel domains and HTTP-5xx/timeout outage simulation. Validation: targeted shared suites green (5 files, 103 tests), client Login and AccountInfo green (31 tests), shared+client tsc --noEmit clean, oxlint 0 errors, lint:vocab clean, en/es/pt parity verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Production-readiness review fixes landed in
Validation: 103 shared + 31 client targeted tests green, shared+client 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/providers/Auth.tsx (1)
448-452:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftExplicit sign-out is no longer durable for passkey users.
This keeps the cached credential, username, and expected smart-account address, but
packages/shared/src/workflows/authServices.tsstill restores a passkey session wheneverauthModeisnull. AftersignOut(), a refresh/remount can silently rehydrate the prior passkey account again, which defeats sign-out on shared devices. Preserve recovery metadata behind a separate signed-out sentinel or separate recovery cache so explicit sign-out suppresses automatic restore until the user opts back in.🤖 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 `@packages/shared/src/providers/Auth.tsx` around lines 448 - 452, Currently signOut only calls clearAuthMode() and clearEmbeddedAddress() but leaves passkey recovery metadata intact so authServices.ts will auto-restore a passkey session when authMode is null; update signOut to persist a "signed-out" sentinel (e.g., setPasskeyRecoveryDisabled or write a signedOutRecovery flag) or move recovery metadata into a separate recovery cache and mark it suppressed, and then modify the restore path in packages/shared/src/workflows/authServices.ts (the routine that rehydrates passkey sessions) to check that sentinel/flag before restoring; ensure any functions reading recovery data (the restore routine) skip recovery while the sentinel is present and provide a clear opt-in path to remove the sentinel when the user explicitly signs back in.
🤖 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 `@packages/shared/src/providers/Auth.tsx`:
- Around line 382-386: The code uses logical OR to derive finalUserName (const
finalUserName = userName || getStoredUsername() || ""), which treats an explicit
empty string from the caller as falsy and incorrectly falls back to stored
username; change this to use nullish coalescing so only undefined/null falls
back: use userName ?? getStoredUsername() ?? "" for finalUserName, and make the
same change for the other username derivation later (the similar
userName/getStoredUsername expression around lines 433-435) so an explicit "" is
preserved and local one-tap routing remains intact.
---
Outside diff comments:
In `@packages/shared/src/providers/Auth.tsx`:
- Around line 448-452: Currently signOut only calls clearAuthMode() and
clearEmbeddedAddress() but leaves passkey recovery metadata intact so
authServices.ts will auto-restore a passkey session when authMode is null;
update signOut to persist a "signed-out" sentinel (e.g.,
setPasskeyRecoveryDisabled or write a signedOutRecovery flag) or move recovery
metadata into a separate recovery cache and mark it suppressed, and then modify
the restore path in packages/shared/src/workflows/authServices.ts (the routine
that rehydrates passkey sessions) to check that sentinel/flag before restoring;
ensure any functions reading recovery data (the restore routine) skip recovery
while the sentinel is present and provide a clear opt-in path to remove the
sentinel when the user explicitly signs back in.
🪄 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: fa16b58c-348f-4816-9a8b-5651d3a08254
📒 Files selected for processing (10)
.plans/active/account-recovery-hardening/plan.todo.md.plans/active/account-recovery-hardening/status.jsonpackages/client/src/__tests__/views/Login.test.tsxpackages/client/src/views/Login/index.tsxpackages/shared/src/__tests__/workflows/authServices.test.tspackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/es.jsonpackages/shared/src/i18n/pt.jsonpackages/shared/src/providers/Auth.tsxpackages/shared/src/workflows/authServices.ts
✅ Files skipped from review due to trivial changes (2)
- .plans/active/account-recovery-hardening/plan.todo.md
- packages/shared/src/i18n/en.json
🚧 Files skipped from review as they are similar to previous changes (6)
- .plans/active/account-recovery-hardening/status.json
- packages/client/src/tests/views/Login.test.tsx
- packages/shared/src/workflows/authServices.ts
- packages/client/src/views/Login/index.tsx
- packages/shared/src/i18n/es.json
- packages/shared/src/i18n/pt.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test
- GitHub Check: CI Gate
- GitHub Check: Playwright Client CI
- GitHub Check: Test
- GitHub Check: Test
- GitHub Check: Storybook
- GitHub Check: Build Docs
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (23)
packages/shared/**
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
@green-goods/sharedis the only home for reusable hooks, providers, stores, modules, types, i18n, and shared UI primitives
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
Use centralized queryKeys, event-driven invalidation, useCurrentChain() or DEFAULT_CHAIN_ID, logger, and typed domain models such as Address in
@green-goods/sharedKeep reusable hooks, providers, stores, modules, and shared UI primitives in
@green-goods/shared
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/shared/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/instructions/shared.instructions.md)
Shared UI primitive changes should ship with tests, barrel updates, and Storybook coverage in the same change when applicable
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Default to single-chain behavior throughgetDefaultChain()orDEFAULT_CHAIN_ID
Useloggerfrom shared instead ofconsole.log
Usebun run format:check && bun lintfor code quality checks
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the
Addresstype for Ethereum addresses instead of raw string types
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Remixicon (
Ri*Line), never lucide for UI icons
Files:
packages/shared/src/providers/Auth.tsx
**/*.{json,ts,tsx}?(locales|i18n|translations|lang)
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Add every new user-facing string to
en,es, andptlanguage files
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/shared/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/shared/AGENTS.md)
packages/shared/**/*.{ts,tsx}: Use centralized query keys fromqueryKeysinpackages/shared. Do not invent ad-hoc query arrays.
UseuseCurrentChain()orDEFAULT_CHAIN_IDfor application defaults inpackages/shared, not wallet chain state.
Prefer event-driven invalidation over polling inpackages/sharedhooks and queries.
Useloggerand typed domain models (Address, discriminated unions,unknownfor untrusted data) inpackages/sharedcode.
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/{shared,client,admin}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never swallow errors — use
parseContractError()+USER_FRIENDLY_ERRORSfor contract errors, usecreateMutationErrorHandler()in shared mutation hooks, useloggerfrom shared (notconsole.log)
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx}: UsequeryKeys.*helpers from shared for React Query key serialization — serialize objects in query keys
Do not use banned vocabulary in i18n strings (enforced bybun run lint:vocab): never usestreak,countdown,leaderboard,FOMO, or growth-hacking language (urgent,limited time,re-engagement,retention hook)
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/shared/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Tailwind v4 does not scan
packages/shared/src/from admin/client builds due to content scanning limitations — do NOT add utility classes (mx-4,w-max,self-center,justify-self-center) directly to JSX in shared components; instead use inline styles for layout or CSS overrides in the consuming package
Files:
packages/shared/src/providers/Auth.tsx
packages/{client,admin,shared}/src/**/*.{tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Design System: Load
designskill (direction) +uiskill (implementation) explicitly when paradigm, layout composition, new view, tokens, or PR review is at stake — for trivial edits (padding, copy, single component touch), use the rules in CLAUDE.md
Files:
packages/shared/src/providers/Auth.tsx
packages/{client,admin,shared}/src/**/*.{css,scss,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Design tokens: never hardcode
cubic-bezier,duration, or raw color/radius values — use--spring-*(6 motion tokens),--color-*,--radius-*,--color-material-*+--blur-material-*from theme.css
Files:
packages/shared/src/providers/Auth.tsx
packages/{contracts,agent,admin,shared}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use the
Addresstype for Ethereum addresses.
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use
loggerfrom shared, neverconsole.log.
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/{client,admin,shared}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Remixicon (
Ri*Line), never lucide.
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/{client,admin,shared}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Any new user-facing string must be added to
en,es, andpt.
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/{client,admin,shared}/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
packages/{client,admin,shared}/src/**/*.{ts,tsx,css}: TreatBaseline Widely Availableas the baseline target for frontend changes. Use repo-installed Modern Web Guidance throughbun run agentic:guidancebefore frontend, UI, CSS, accessibility, browser proof, or web-design changes.
Prefer semantic HTML, native controls, platform CSS, and browser primitives before custom JavaScript.
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
packages/{client,admin,shared}/src/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Banned vocabulary for any surface:
streak,countdown,leaderboard,FOMO.
Files:
packages/shared/src/providers/Auth.tsxpackages/shared/src/__tests__/workflows/authServices.test.ts
{.github/workflows/**,**/*.test.{js,ts,jsx,tsx},**/*.spec.{js,ts,jsx,tsx}}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
bun run test, neverbun test
Files:
packages/shared/src/__tests__/workflows/authServices.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
bun run testfor running tests
Files:
packages/shared/src/__tests__/workflows/authServices.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bun run testinstead ofbun testfor running tests —bun testuses bun's built-in runner (ignores vitest config), whilebun run testruns the package.json script with proper vitest environment
Files:
packages/shared/src/__tests__/workflows/authServices.test.ts
packages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
Addresstype (notstring) for Ethereum addresses in TypeScript across all packages
Files:
packages/shared/src/__tests__/workflows/authServices.test.ts
| // Get stored username or use provided. An empty username routes the | ||
| // auth service straight to the local one-tap path — never fabricate a | ||
| // placeholder, because under the passkey-server flag any truthy name | ||
| // triggers a hosted-server lookup for that literal username. | ||
| const finalUserName = userName || getStoredUsername() || ""; |
There was a problem hiding this comment.
Preserve an explicit empty username here.
userName || getStoredUsername() || "" turns a caller-supplied "" back into the cached username, so a blank recovery field no longer reaches the intended local one-tap path. Under the server flag that can send a stale recovery name to hosted lookup instead of staying local. Use nullish coalescing so only undefined falls back to storage.
Suggested fix
- const finalUserName = userName || getStoredUsername() || "";
+ const finalUserName = userName ?? getStoredUsername() ?? "";- const finalUserName = userName || getStoredUsername() || "";
+ const finalUserName = userName ?? getStoredUsername() ?? "";Also applies to: 433-435
🤖 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 `@packages/shared/src/providers/Auth.tsx` around lines 382 - 386, The code uses
logical OR to derive finalUserName (const finalUserName = userName ||
getStoredUsername() || ""), which treats an explicit empty string from the
caller as falsy and incorrectly falls back to stored username; change this to
use nullish coalescing so only undefined/null falls back: use userName ??
getStoredUsername() ?? "" for finalUserName, and make the same change for the
other username derivation later (the similar userName/getStoredUsername
expression around lines 433-435) so an explicit "" is preserved and local
one-tap routing remains intact.
1 participant
Summary
VITE_PASSKEY_SERVER_ENABLED, defaulting off for staging-first rollout and rollback.Validation
bun run --cwd packages/shared test -- src/__tests__/config/passkeyServer.test.ts src/__tests__/workflows/authServices.test.ts src/__tests__/modules/session.test.ts src/__tests__/workflows/authMachine.test.ts src/__tests__/hooks/useAuth.test.tsbun run --cwd packages/client test -- src/__tests__/views/Login.test.tsxbun run build:sharedbun run --cwd packages/client buildbun run lint:vocabbun run check:design-mdbun run check:design-tokenshttps://127.0.0.1:5173/home/login?presentation=pwaPre-push checks passed.Known unrelated validation blockers:
bun run check:design-generatedfails on stale generated client PWA token audit docs.node scripts/harness/plan-hub.mjs validatefails on malformed activesentry-stack-observabilityhub metadata.Linear: PRD-537, PRD-538, PRD-540, PRD-505, PRD-507