fix: persist post-auth valuation intent to sessionStorage so the run resumes after the OTP#45
Open
sweetmantech wants to merge 1 commit into
Open
fix: persist post-auth valuation intent to sessionStorage so the run resumes after the OTP#45sweetmantech wants to merge 1 commit into
sweetmantech wants to merge 1 commit into
Conversation
…resumes after the OTP useCatalogValuation kept the deferred "run after login" artist in a React ref, auto-fired on the authenticated flip. That works when the component survives auth, but fresh-signup auth churn remounts it and the ref dies with the instance, dropping the intent (chat#1850 2026-07-08 audit). Persist the picked artist (+ timestamp) to a versioned sessionStorage key at the auth gate and resume from storage on the authenticated effect, which also covers a full page reload mid-auth. The ref stays as the fast path; storage is the source of truth. Intents older than 15 minutes are ignored; clearPick drops the persisted copy too. Helpers TDD'd (RED first) in lib/valuation with logic-only vitest coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
23 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis change introduces a sessionStorage-backed pending intent mechanism for the catalog valuation flow. New ChangesPersisted Pending Intent
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant useCatalogValuation
participant sessionStorage
participant AuthProvider
User->>useCatalogValuation: run() while signed out
useCatalogValuation->>sessionStorage: savePendingIntent(artist)
useCatalogValuation->>AuthProvider: login()
AuthProvider-->>useCatalogValuation: authenticated
useCatalogValuation->>sessionStorage: readPendingIntent()
sessionStorage-->>useCatalogValuation: artist
useCatalogValuation->>sessionStorage: clearPendingIntent()
useCatalogValuation->>useCatalogValuation: doRun(artist)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/valuation/readPendingIntent.ts">
<violation number="1" location="lib/valuation/readPendingIntent.ts:26">
P2: Malformed timestamps can bypass the expiry guard and still resume a valuation intent. The `savedAt` check only verifies `number`, so `NaN`/`Infinity` pass; adding a finite-number guard keeps malformed storage data from being accepted.</violation>
</file>
<file name="hooks/useCatalogValuation.ts">
<violation number="1" location="hooks/useCatalogValuation.ts:87">
P2: Stale signed-out intents can still auto-run when the component instance survives auth, because `pendingRun.current` bypasses the 15-minute expiry enforced only by `readPendingIntent()`. Consider storing `{ artist, savedAt }` in the ref or validating the ref against the same max-age before preferring it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (typeof artist?.id !== "string" || typeof artist.name !== "string") { | ||
| return null; | ||
| } | ||
| if (typeof savedAt !== "number") return null; |
There was a problem hiding this comment.
P2: Malformed timestamps can bypass the expiry guard and still resume a valuation intent. The savedAt check only verifies number, so NaN/Infinity pass; adding a finite-number guard keeps malformed storage data from being accepted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/valuation/readPendingIntent.ts, line 26:
<comment>Malformed timestamps can bypass the expiry guard and still resume a valuation intent. The `savedAt` check only verifies `number`, so `NaN`/`Infinity` pass; adding a finite-number guard keeps malformed storage data from being accepted.</comment>
<file context>
@@ -0,0 +1,32 @@
+ if (typeof artist?.id !== "string" || typeof artist.name !== "string") {
+ return null;
+ }
+ if (typeof savedAt !== "number") return null;
+ if (now - savedAt > PENDING_INTENT_MAX_AGE_MS) return null;
+ return artist;
</file context>
| void doRun(artist); | ||
| } | ||
| if (!authenticated) return; | ||
| const artist = pendingRun.current ?? readPendingIntent(); |
There was a problem hiding this comment.
P2: Stale signed-out intents can still auto-run when the component instance survives auth, because pendingRun.current bypasses the 15-minute expiry enforced only by readPendingIntent(). Consider storing { artist, savedAt } in the ref or validating the ref against the same max-age before preferring it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useCatalogValuation.ts, line 87:
<comment>Stale signed-out intents can still auto-run when the component instance survives auth, because `pendingRun.current` bypasses the 15-minute expiry enforced only by `readPendingIntent()`. Consider storing `{ artist, savedAt }` in the ref or validating the ref against the same max-age before preferring it.</comment>
<file context>
@@ -65,20 +71,25 @@ export function useCatalogValuation(): CatalogValuationState {
- void doRun(artist);
- }
+ if (!authenticated) return;
+ const artist = pendingRun.current ?? readPendingIntent();
+ pendingRun.current = null;
+ clearPendingIntent(); // consume-once: never re-fire on later mounts
</file context>
This was referenced Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ships the post-auth intent drop item of recoupable/chat#1850: "Value my catalog" did not resume after the OTP for fresh signups; the lead had to click it a second time.
Root cause
Per the 2026-07-08 audit: a resume mechanism already existed.
useCatalogValuationstored the picked artist in apendingRunref and auto-fireddoRunon theauthenticatedflip. That works when the component instance survives auth (the 07-06 walkthrough auto-resumed through it), but fresh-signup auth flows churn enough state to remount the component mid-auth, and a ref dies with its instance: the intent is silently dropped, leaving the idle homepage observed in the 07-07 walkthrough.Fix
Persist the intent to
sessionStorageat the auth gate and resume from storage. Storage is the source of truth; the ref remains as the fast path.run()while signed out):savePendingIntent(picked)writes{artist, savedAt}under the versioned keyrecoupable-valuation-pending-intent:v1beforelogin().authenticatedeffect now also runs on mount when already authenticated, so it covers the remount AND full-reload cases. It takespendingRun.current ?? readPendingIntent(), clears both (consume-once, never re-fires on later mounts), restorespicked(a remount lost it, and the result card renders the artist), and firesdoRun.PENDING_INTENT_MAX_AGE_MS) are ignored, so a stale tab never auto-runs a valuation.clearPick()clears the persisted copy along with the ref, preserving the existing drop-the-deferred-run semantic.New helpers in
lib/valuation/(one exported function per file):savePendingIntent,readPendingIntent,clearPendingIntent, pluspendingIntent.ts(shared key/max-age constants + type). All storage access is SSR-guarded and try/caught (private-mode failures fall back to the ref path).Tests
TDD, RED first: 13 new logic-only vitest cases (3 save, 8 read, 2 clear) against a Map-backed
sessionStoragestub. Full suite: 80 passed (was 67).pnpm buildgreen.Verification
pnpm test— 80/80pnpm build— production build greenrecoupable.dev; will be recorded as a PR comment per the "Done when" on chat#1850.🤖 Generated with Claude Code
Summary by cubic
Fixes the post-auth “Value my catalog” flow so the run resumes automatically after OTP, including fresh signup flows that remount the component (chat#1850). The deferred intent is now saved to sessionStorage and consumed on login.
recoupable-valuation-pending-intent:v1.clearPick()also removes the persisted intent. Added unit tests for save/read/clear helpers.Written for commit 539e156. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests