refactor(wallet): remove stale isSignedInWithTrial flag - #3563
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughChangesTrial wallet flow removal
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM. Clean removal of a dead, permanently-stale flag with matching test/behavior updates.
Extended reasoning...
Overview
The PR deletes the isSignedInWithTrial jotai atom, its only writer (a useEffect in useManagedWallet), and its two read sites (WalletConnectionButtons and NoDeploymentsState). Tests are updated to match: the jotai store/provider plumbing and the removed 'Sign in' link assertion are dropped from WalletConnectionButtons.spec.tsx.
Security risks
None. This only removes a client-side, localStorage-persisted UI flag; no auth, data access, or validation logic is touched.
Level of scrutiny
Low. This is a straightforward dead-code removal in the frontend UI layer (deploy-web), not a critical path. I grepped the repo for remaining references to isSignedInWithTrial and WalletConnectionButtons usage — no other call sites depend on the removed prop/behavior, and the PR description's stated rationale (write-only-to-true, never reset, superseded by onboarding flow) checks out against the diff.
Other factors
The behavior change (always showing 'Start Trial' instead of conditionally showing a 'Sign in' link) is intentional and explained in the PR description, and the spec file was updated consistently. Scope is small (5 files) and self-contained.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3563 +/- ##
==========================================
- Coverage 75.39% 74.53% -0.86%
==========================================
Files 1165 1073 -92
Lines 30316 27928 -2388
Branches 7540 7064 -476
==========================================
- Hits 22856 20817 -2039
+ Misses 6587 6268 -319
+ Partials 873 843 -30
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/deploy-web/src/components/wallet/WalletConnectionButtons.spec.tsx`:
- Around line 11-19: Replace both vi.fn(ComponentMock) constructions in the test
and setup function with typed mock<typeof
DEPENDENCIES.ConnectManagedWalletButton>() instances from vitest-mock-extended,
configuring ComponentMock as the mock implementation so the component still
returns JSX. Remove the resulting as unknown as cast while preserving the
existing dependency injection and assertion behavior.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66beb5db-5809-4e6e-b506-58a8310da4d0
📒 Files selected for processing (5)
apps/deploy-web/src/components/home/NoDeploymentsState.tsxapps/deploy-web/src/components/wallet/WalletConnectionButtons.spec.tsxapps/deploy-web/src/components/wallet/WalletConnectionButtons.tsxapps/deploy-web/src/hooks/useManagedWallet.tsapps/deploy-web/src/store/walletStore.ts
💤 Files with no reviewable changes (3)
- apps/deploy-web/src/hooks/useManagedWallet.ts
- apps/deploy-web/src/components/home/NoDeploymentsState.tsx
- apps/deploy-web/src/store/walletStore.ts
There was a problem hiding this comment.
LGTM — the removed "Start Trial"/"Connect Wallet" copy nit below is minor and doesn't block this cleanup.
Extended reasoning...
Overview
This PR deletes the now-dead "Start Trial"/manual wallet-connect UI surface (ConnectManagedWalletButton, WalletConnectionButtons, the isSignedInWithTrial jotai atom and its writer effect) since wallets and trials are now auto-provisioned on account creation. It touches WalletStatus, NoDeploymentsState, GetStartedStepper, DeploymentList, and useManagedWallet, removing unused props/branches and updating the corresponding specs.
Security risks
None. This is a pure UI/state cleanup removing dead code paths — no auth, data access, or crypto logic is touched.
Level of scrutiny
Low-to-moderate is appropriate: it's a mechanical deletion of an obsolete flow with no behavioral risk to the remaining paths (managed wallet creation/usage is unchanged), and Codecov confirms all changed/coverable lines remain covered with all tests passing. The one flagged issue is stale instructional copy left behind by the removal — cosmetic, not a functional break, and callable out via the inline comment rather than blocking the PR.
Other factors
Tests were updated consistently with the component deletions (specs removed alongside components, WalletStatus spec updated to assert nothing renders). No outstanding unaddressed reviewer comments other than CodeRabbit's optional test-typing nit, which is stylistic and not correctness-affecting.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/components/get-started/GetStartedStepper.tsx (1)
22-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the disconnected-wallet instructions.
On
/get-started,WalletStatusrenders nothing when disconnected, and theStart Trialbutton exists only inPaymentMethodStep. Users cannot click either"Start Trial"or"Connect Wallet"from this page. Remove the instruction or render matching controls.🤖 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 `@apps/deploy-web/src/components/get-started/GetStartedStepper.tsx` around lines 22 - 27, Update the disconnected-wallet guidance in GetStartedStepper so it does not instruct users to click unavailable “Start Trial” or “Connect Wallet” controls when WalletStatus renders nothing and PaymentMethodStep is not active; remove that instruction or render controls matching those actions.
🤖 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.
Outside diff comments:
In `@apps/deploy-web/src/components/get-started/GetStartedStepper.tsx`:
- Around line 22-27: Update the disconnected-wallet guidance in
GetStartedStepper so it does not instruct users to click unavailable “Start
Trial” or “Connect Wallet” controls when WalletStatus renders nothing and
PaymentMethodStep is not active; remove that instruction or render controls
matching those actions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85a547b4-83b3-4959-9c49-c9cd34ae8a08
📒 Files selected for processing (9)
apps/deploy-web/src/components/deployments/DeploymentList.tsxapps/deploy-web/src/components/get-started/GetStartedStepper.tsxapps/deploy-web/src/components/home/NoDeploymentsState.tsxapps/deploy-web/src/components/layout/WalletStatus.spec.tsxapps/deploy-web/src/components/layout/WalletStatus.tsxapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsxapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsxapps/deploy-web/src/components/wallet/WalletConnectionButtons.spec.tsxapps/deploy-web/src/components/wallet/WalletConnectionButtons.tsx
💤 Files with no reviewable changes (4)
- apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.spec.tsx
- apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsx
- apps/deploy-web/src/components/wallet/WalletConnectionButtons.tsx
- apps/deploy-web/src/components/wallet/WalletConnectionButtons.spec.tsx
There was a problem hiding this comment.
LGTM — the stale copy issue I flagged in my previous review (GetStartedStepper still referencing the removed "Start Trial"/"Connect Wallet" buttons) has been fixed by a follow-up commit removing that paragraph; no other issues found.
Extended reasoning...
This PR completes a dead-code removal refactor: it deletes the obsolete isSignedInWithTrial jotai atom and its only writer, deletes the now-unused ConnectManagedWalletButton and WalletConnectionButtons components (and their specs), and simplifies WalletStatus, NoDeploymentsState, DeploymentList, and GetStartedStepper to stop rendering the removed connect-wallet UI. Grep confirms no remaining references to isSignedInWithTrial, WalletConnectionButtons, or ConnectManagedWalletButton anywhere in apps/deploy-web/src, so the removal is clean. No security risk: this is UI dead-code removal in a non-auth, non-crypto surface. Scrutiny level is low-to-moderate given the diff is almost entirely deletions plus straightforward simplification, covered by updated specs. In a prior run I flagged that GetStartedStepper left a paragraph referencing the deleted Start Trial/Connect Wallet buttons; a follow-up commit in this diff removes that paragraph, resolving the concern.
2 participants
Why
The
isSignedInWithTrialjotai atom (localStorage-persisted, introduced Oct 2024) tracked "this browser once had a signed-in user with a trial wallet". It was write-only-to-true— never reset on logout, never cleared — so it went permanently stale.More broadly, the entire "Start Trial" connect-wallet surface is obsolete: a managed wallet and trial are now created automatically when an account is created, so there is no manual wallet-connection step left for the UI to offer.
What
isSignedInWithTrialatom fromwalletStoreand its only writer (auseEffectinuseManagedWallet).ConnectManagedWalletButton("Start Trial") andWalletConnectionButtonscomponents + specs — dead UI now that wallets are auto-provisioned.WalletStatus(top nav / mobile sidebar) renders nothing when no wallet is connected instead of the Start Trial button.NoDeploymentsStatealways shows the Create Deployment / Explore Templates CTAs; theisWalletConnectedprop and the "you may need to sign in" hint are removed.GetStartedStepperkeeps the "Billing is not set up" indicator but no longer offers the connect buttons.The orphaned
isSignedInWithTriallocalStorage key in existing browsers is harmless and needs no cleanup migration.Summary by CodeRabbit
Simplified Deployment Experience
Onboarding Updates
Bug Fixes