Chore: Refactor Story Creation Flow#443
Open
suhaib-ilahi wants to merge 3 commits into
Open
Conversation
|
@suhaib-ilahi is attempting to deploy a commit to the Drago's projects Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
Refactor
Contributor
📝 WalkthroughWalkthroughA new client-side, 5-step Creation Wizard replaces fragmented creation UI: mode selection, core prompt, advanced params, preview, and publish (free or NFT). Adds autosave/draft recovery, local + backend sync, analytics, and wallet-aware publish flows; several pages were simplified to delegate to the wizard. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as CreationWizard (Client)
participant State as useCreationWizard (Local State)
participant Analytics as useWizardAnalytics (Tracking)
participant API as Server API
participant Storage as LocalStorage / Draft Store
User->>Client: Open wizard / choose entry params
Client->>State: initialize(account, initialGenre, format)
State->>Storage: restore local snapshot
State->>API: GET /api/v1/drafts (load remote draft)
API-->>State: draft payload / metadata
State-->>Client: recovered draft available
User->>Client: Select mode / enter prompt / set params
Client->>Analytics: track events (mode, genre, prompt)
Client->>State: setMode / setCorePrompt / setAdvancedParams
State->>Storage: persist snapshot (autosave)
State->>API: PUT /api/v1/drafts (autosave sync)
User->>Client: Generate content
Client->>API: POST /api/generate-story
API-->>Client: generated content
Client->>State: setGeneratedContent
Client->>Analytics: onContentGenerated
User->>Client: Publish (free or NFT)
Client->>Client: prepare metadata payload
Client->>API: POST IPFS upload (metadata)
API-->>Client: ipfsHash
alt NFT path
Client->>User: prompt wallet connect
Client->>API: mint transaction
API-->>Client: mint result
else Free story
Client->>API: save story metadata
end
Client->>Analytics: onPublishSucceeded
State->>Storage: clear draft
Client->>User: show success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (10)
hooks/use-wizard-analytics.ts (2)
77-86: Prevent callers from overridingstepLabelin metadata merge.Right now
metadatacan replacestepLabel. If you wantstepLabelto be authoritative, merge in the other order.Proposed diff
- metadata: { - stepLabel: STEP_LABELS[step], - ...metadata, - }, + metadata: { + ...metadata, + stepLabel: STEP_LABELS[step], + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wizard-analytics.ts` around lines 77 - 86, The payload construction allows callers to override stepLabel because metadata is spread after the hardcoded value; update the merge so stepLabel from STEP_LABELS[step] is authoritative by merging metadata first and then setting stepLabel (i.e., ensure metadata is spread before assigning stepLabel) when building the metadata field for the AnalyticsPayload in the code that constructs payload (refer to payload, AnalyticsPayload, STEP_LABELS, step, metadata, sessionIdRef).
94-107: Consider afetch(..., { keepalive: true })fallback whensendBeaconis unavailable.Some environments (or privacy settings) disable
sendBeacon, which will currently drop analytics entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wizard-analytics.ts` around lines 94 - 107, Add a non-blocking fetch fallback that uses fetch('/api/analytics', { method: 'POST', body: JSON.stringify(payload), headers: {'Content-Type':'application/json'}, keepalive: true }) when navigator.sendBeacon is not available or fails; update the send block in hooks/use-wizard-analytics.ts (the navigator.sendBeacon try/catch around payload creation) to call this fallback (wrapped in try/catch and not awaited) so analytics are still delivered in environments that disable sendBeacon while preserving non-blocking behavior.components/wizard/step-core-prompt.tsx (2)
52-57: Make the error banner announceable (role="alert"/aria-live).This improves accessibility for validation errors (screen readers will announce changes).
Proposed diff
{errors && ( - <div className="border border-destructive bg-destructive/10 p-3 rounded-md text-sm font-medium text-destructive"> + <div + role="alert" + aria-live="polite" + className="border border-destructive bg-destructive/10 p-3 rounded-md text-sm font-medium text-destructive" + > {errors} </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wizard/step-core-prompt.tsx` around lines 52 - 57, The error banner rendering for {errors} in the component (the conditional div in step-core-prompt.tsx) needs ARIA for screen-reader announcement: update the error div that displays {errors} to include role="alert" and/or aria-live="assertive" (and optionally aria-atomic="true") so validation messages are announced; keep the same conditional rendering referencing errors and only add these attributes to that div.
135-140: Use the blur event value for prompt-length analytics (avoid any stale-prop edge case).Right now the blur handler reads
data.prompt, which can undercount in edge cases. Using the textarea’s value is more direct.Proposed diff
- onBlur={() => { - onBlur?.(); - if (data.prompt.trim()) { - onPromptAnalytics?.(data.prompt.trim().length); - } - }} + onBlur={(e) => { + onBlur?.(); + const trimmed = e.currentTarget.value.trim(); + if (trimmed) onPromptAnalytics?.(trimmed.length); + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wizard/step-core-prompt.tsx` around lines 135 - 140, The blur handler currently reads data.prompt and may use stale props; update the onBlur handler for the textarea to use the event's current value (e.g. e.currentTarget.value) instead of data.prompt when computing trimmed length for analytics: keep calling onBlur?.(), then compute const val = (e.currentTarget.value || "").trim() and call onPromptAnalytics?.(val.length) only if val is non-empty. Locate the handler attached to the textarea element where onBlur, onPromptAnalytics, and data.prompt are referenced and replace the data.prompt usage with the event value.components/wizard/step-publish.tsx (1)
185-242: Freeze publish-format changes during publishing, and gate the Publish button on the actual content you upload.Right now users can toggle “Free/NFT” while
publishStateis mid-flight, and the button disables ongeneratedContenteven though metadata usesgeneratedContent || corePrompt.prompt.Proposed diff
const isNft = publishFormat === 'nft'; + const publishDisabled = + publishState !== 'idle' || + !(generatedContent?.trim() || corePrompt.prompt.trim()) || + (isNft && !connected); ... <button type="button" aria-pressed={!isNft} + disabled={publishState !== 'idle'} onClick={() => onPublishFormatChange('free')} ... <button type="button" aria-pressed={isNft} + disabled={publishState !== 'idle'} onClick={() => onPublishFormatChange('nft')} ... {publishState === 'idle' && ( <div className="flex justify-center pt-2"> <Button size="lg" className={cn( 'w-full h-14 text-lg font-bold tracking-wide transition-all', - !generatedContent?.trim() || (isNft && !connected) + publishDisabled ? 'bg-muted text-muted-foreground cursor-not-allowed' : 'theme-gradient-bg text-white hover:opacity-90' )} - disabled={!generatedContent?.trim() || (isNft && !connected)} + disabled={publishDisabled} onClick={handlePublish} >Also applies to: 327-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wizard/step-publish.tsx` around lines 185 - 242, Prevent changing publish format while a publish is in progress by disabling the Free/NFT buttons when publishState indicates publishing; update the button props that use isNft and onPublishFormatChange (the two <button> elements controlling publish format) to include disabled={publishState === 'publishing' || publishState === 'inflight'} and short-circuit onPublishFormatChange to return early if publishState is publishing. Also gate the Publish button enablement to check actual uploaded content rather than generatedContent alone by changing its disabled condition to consider uploadedContent (or files) and use metadataContent = generatedContent || corePrompt.prompt where appropriate so the metadata still falls back to corePrompt.prompt but the Publish action requires uploadedContent to be present.hooks/use-creation-wizard.ts (2)
169-181: Make Step 4 validation consistent with the snapshot “content source of truth”.You snapshot
content: state.generatedContent || state.corePrompt.prompt, but Step 4 validation only allowsgeneratedContent. If “paste your own content in Step 2” is intended, consider validating on(generatedContent || corePrompt.prompt).trim().length > 0and aligning StepPublish gating similarly.Also applies to: 285-294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-creation-wizard.ts` around lines 169 - 181, getValidation currently marks step4 valid only when state.generatedContent has text, but your snapshot uses content: state.generatedContent || state.corePrompt.prompt; change step4's check to validate (s.generatedContent || s.corePrompt.prompt).trim().length > 0 so pasted-in prompt counts as content. Update the corresponding StepPublish gating logic (the code around the StepPublish checks referenced in the comment) to use the same (generatedContent || corePrompt.prompt) trimmed presence check so both validations are consistent; locate symbols getValidation, stateRef, WizardValidation, step4, state.generatedContent and state.corePrompt.prompt to apply the change.
298-343: Optional: protect backend sync from overlapping calls.
isSyncingcan flicker incorrectly if autosave/blur triggers overlap, and you may end up with concurrent PUTs. A simple “in-flight” ref (skip/queue) or aborting previous syncs would make this more predictable.Also applies to: 346-394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-creation-wizard.ts` around lines 298 - 343, The syncDraftToBackend function can start overlapping PUTs causing isSyncing flicker; fix by adding an in-flight guard or by aborting any previous request before starting a new one: create a ref (e.g., syncInFlightRef or pendingAbortRef) outside syncDraftToBackend and, at the top of syncDraftToBackend, either return early if syncInFlightRef.current is true or call pendingAbortRef.current?.abort() to cancel the previous ctrl; then set syncInFlightRef.current = true (or store the new AbortController in pendingAbortRef.current) before fetch and clear/reset that ref in the finally block (and still clearTimeout tid), ensuring isSyncing state reflects only the true active request and preventing concurrent PUTs.components/wizard/step-mode-select.tsx (2)
68-91: Avoid manual Enter/Space handling on a<button>(and dedupe selection logic).Native buttons already activate on Enter/Space; the custom
onKeyDownrisks double-callingonSelect/onAnalyticsdepending on browser/AT behavior and is duplicated withonClick.Proposed diff
+ const selectMode = () => { + onSelect(mode.id); + onAnalytics?.(mode.id); + }; + <button type="button" aria-pressed={isSelected} - onClick={() => { - onSelect(mode.id); - onAnalytics?.(mode.id); - }} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - onSelect(mode.id); - onAnalytics?.(mode.id); - } - }} + onClick={selectMode} className={cn( 'w-full h-auto p-6 flex flex-col items-center gap-5 text-center', 'border-4 border-foreground bg-card shadow-[6px_6px_0px_0px_hsl(var(--foreground))]', 'hover:-translate-y-1 hover:shadow-[8px_8px_0px_0px_hsl(var(--foreground))]', 'active:translate-y-0 active:shadow-none', 'transition-all cursor-pointer', + 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2 focus-visible:ring-offset-background', isSelected && 'bg-primary/10 border-primary ring-2 ring-primary ring-inset' )} >Also applies to: 75-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wizard/step-mode-select.tsx` around lines 68 - 91, Remove the manual onKeyDown handler from the <button> in components/wizard/step-mode-select.tsx to avoid double-invoking selection and analytics (native buttons already handle Enter/Space); keep the existing onClick that calls onSelect(mode.id) and onAnalytics?.(mode.id). If you need custom keyboard behavior, implement it only for non-button elements or centralize the selection logic into a single helper (e.g., callSelect(mode.id) used by onClick) so selection/analytics invocation is not duplicated elsewhere in this component.
5-5: Remove the no-opuseEffectand tightenonAnalyticstyping.The empty effect doesn’t do anything and implies side effects that aren’t there. Also
onAnalyticscan be typed asStoryModeto matchonSelect.Proposed diff
-import React, { useEffect } from 'react'; +import React from 'react'; interface StepModeSelectProps { selectedMode: StoryMode | null; onSelect: (mode: StoryMode) => void; - onAnalytics?: (mode: string) => void; + onAnalytics?: (mode: StoryMode) => void; } export function StepModeSelect({ selectedMode, onSelect, onAnalytics, }: StepModeSelectProps) { - useEffect(() => { - // analytics handled by parent - }, []); - return (Also applies to: 10-14, 43-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wizard/step-mode-select.tsx` at line 5, Remove the no-op useEffect import and any empty useEffect calls in components/wizard/step-mode-select.tsx (they serve no purpose); then tighten the typing of the onAnalytics prop and any handlers currently using a generic type to use the StoryMode type (to match onSelect) so onAnalytics receives a StoryMode parameter; update the prop signature and any internal function declarations that reference onAnalytics or onSelect accordingly.components/wizard/step-advanced-params.tsx (1)
73-111: Associate<Label>with controls (ids/htmlFor) for better accessibility.Some labels aren’t linked to their SelectTrigger/Input, which reduces SR usability and click-to-focus behavior.
Example pattern
- <Label>Art Style</Label> + <Label htmlFor="wizard-art-style">Art Style</Label> <Select value={params.artStyle} onValueChange={(v) => handleChange('artStyle', v)} > - <SelectTrigger> + <SelectTrigger id="wizard-art-style"> <SelectValue placeholder="Select art style" /> </SelectTrigger>(Repeat for other fields.)
Also applies to: 199-215, 231-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wizard/step-advanced-params.tsx` around lines 73 - 111, The Label components for the Art Style and Panel Layout fields are not associated with their Select controls; update each field to add a unique id (e.g., artStyle-select and panelLayout-select) on the Select/SelectTrigger or the underlying input element that the Select library exposes, and set the corresponding Label's htmlFor to that id so clicking the label focuses the Select. Apply the same pattern for other fields referenced (lines ~199-215 and ~231-285), keeping ids unique and preserving existing props like value (params.artStyle, params.panelLayout) and onValueChange (handleChange).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/create/ai-story/page.tsx`:
- Line 36: The code currently trusts user input by asserting const format =
(searchParams.get('format') || 'free') as 'free' | 'nft'; — change this to a
runtime-validated value: read rawFormat = searchParams.get('format'), check if
rawFormat === 'free' or rawFormat === 'nft', and only then set format to that
value; otherwise fall back to 'free'. Update the usage site(s) of the format
variable in this file (page.tsx) so they consume the validated format variable
instead of the unchecked assertion.
In `@app/create/page.tsx`:
- Around line 21-23: The code uses a client-side localStorage flag via
localStorage.getItem('adminSession') assigned to isAdmin to gate admin access
alongside account, which is an auth bypass vector; replace this check with a
server-verified admin check by calling a backend API or relying on a
server-provided httpOnly session/token before rendering or taking admin actions.
Concretely, remove use of localStorage.getItem('adminSession') and instead call
an API endpoint (or use server-side props/session) to validate admin status, use
the returned server-validated admin boolean in place of isAdmin where the
current guard (if (!account && !isAdmin) ...) runs, and surface failures via the
existing toast path after the server validation. Ensure the unique symbols
touched are isAdmin, localStorage.getItem('adminSession'), and the conditional
that checks account to prevent trusting client-set values.
- Around line 22-34: The denial branch in the useEffect early-returns before
clearing loading, so move or add a call to setIsLoading(false) (and optionally
setIsAuthorised(false)) before router.push('/')/return to ensure loading state
is always cleared; locate the hook containing setIsAuthorised, setIsLoading,
account, isAdmin and router (the useEffect in page.tsx) and update the
denied-path to call setIsLoading(false) prior to returning.
In `@components/wizard/creation-wizard.tsx`:
- Line 311: The Next button's JSX has a malformed className string: remove the
stray double-quote inside the template literal so the token `text-sm` is not
broken and ensure the conditional concat using wizard.canGoNext() remains inside
the template literal; update the className expression (the Next button's
className) to produce a single valid string, e.g., `comic-button-secondary flex
items-center text-sm ${!wizard.canGoNext() ? 'opacity-50 cursor-not-allowed' :
''}`.
- Around line 208-213: The mode-selection handler is invoked twice because
analytics.onModeSelected is called both inside the onSelect callback and again
via the onAnalytics prop; remove the duplicate by having onSelect only call
wizard.setMode (remove analytics.onModeSelected from the onSelect body) and keep
the onAnalytics={analytics.onModeSelected} prop so the child emits analytics
once. Adjust references to wizard.setMode and analytics.onModeSelected
accordingly.
In `@components/wizard/step-advanced-params.tsx`:
- Around line 39-42: Change handleChange to a generic so the value is typed to
the specific AdvancedParams property: declare it as function handleChange<K
extends keyof AdvancedParams>(key: K, value: AdvancedParams[K]) and pass the
updated partial to onChange using a correctly typed object (e.g. onChange({
[key]: value } as Pick<AdvancedParams, K>) or as Partial<AdvancedParams>), then
call onParamAnalytics?.(key, value) as before; this ensures type-safety for
AdvancedParams properties while keeping existing behavior of handleChange,
onChange and onParamAnalytics.
In `@components/wizard/step-preview.tsx`:
- Around line 160-213: The preview currently uses a truthy check on
generatedContent which treats an empty string as “no content” and collapses the
editor; change the conditional that renders the preview from a truthy check to
one that treats empty string as valid when editing — e.g. replace the top-level
check using generatedContent (in the JSX around the preview rendering) with a
condition like (generatedContent !== null && generatedContent !== undefined) ||
isEditing so the Textarea (component Textarea, value prop passed as
generatedContent) stays visible while editing; also ensure places consuming
generatedContent use a safe fallback (generatedContent ?? '') where needed.
- Around line 64-85: The component currently posts payload from variables
(corePrompt, mode, advancedParams) to a non-existent endpoint via
fetch('/api/generate-story'), causing 404s; update the request to call
'/api/groq' and transform the payload to include action: 'generate', map
storyLength → length, and nest advancedParams under an options object (e.g.,
options: advancedParams) so the contract matches the /api/groq expectation, then
keep parsing the response using data.result and pass the extracted content to
onContentChange; alternatively, if you prefer creating a new endpoint, implement
/api/generate-story that accepts the existing payload shape and returns { result
} to preserve current client code.
- Around line 126-131: The error banner rendering currently lacks ARIA
attributes and the cover preview uses next/image which doesn't support blob
URLs; update the error banner JSX that renders {errors} to include role="alert"
and aria-live="polite" (the same conditional block that checks errors), and
replace the use of the Image component that uses coverImagePreview with a plain
<img> element bound to coverImagePreview (preserve alt, className, and
sizing/props currently applied to the Image) so blob URLs produced by
URL.createObjectURL(file) work in Next.js 14.1.0.
In `@components/wizard/step-publish.tsx`:
- Around line 58-81: The getIpfsClient function in the client component leaks
secrets (NEXT_PUBLIC_INFURA_IPFS_PROJECT_SECRET), uses Buffer in the browser and
silently returns mock hashes; instead, implement a server-side API endpoint
(e.g., /api/ipfs/upload) that uses non-public env vars (INFURA_IPFS_PROJECT_ID
and INFURA_IPFS_PROJECT_SECRET) to instantiate the ipfs-http-client and call its
add method, then return the real CID; update the client to remove getIpfsClient
and instead POST file/data to that API and surface errors (no silent mock
fallback), ensuring any fallback logic is server-side and the client only
handles success/error responses.
- Around line 92-97: The code stores res.path (the filename) instead of the IPFS
content identifier; update the two places where coverHash (and the other IPFS
file-hash variable) are set after calling ipfs.add(...) to use
res.cid.toString() instead of res.path so the canonical CID is saved (locate
occurrences around the coverImageFile handling and the other ipfs.add call and
replace res.path with res.cid.toString()).
In `@components/wizard/wizard-stepper.tsx`:
- Around line 54-83: The button currently strips default focus styles via the
'outline-none' class and the active circle uses 'bg-primary' without setting a
contrasting text color; update the button's className (where 'button' element
and its props like onStepClick, isAccessible are defined) to replace
'outline-none' with accessible focus utilities (e.g., add 'focus-visible:ring-2
focus-visible:ring-offset-2 focus-visible:ring-primary' or equivalent) so
keyboard focus is visible, and update the motion.div class construction (the
block using isActive/isCompleted/isAccessible) to include a contrasting text
color when isActive (e.g., add 'text-primary-foreground' or the appropriate
foreground token alongside 'bg-primary') so active-step text meets contrast
requirements.
In `@hooks/use-creation-wizard.ts`:
- Around line 169-201: The restore flow can leave corePrompt.prompt populated
but generatedContent null, causing getValidation().step4 and getErrors().step4
to block progression; update the logic so step4 treats either
stateRef.current.generatedContent (non-empty) OR
stateRef.current.corePrompt.prompt (non-empty) as valid, and mirror that change
in getErrors to return null when either field has content; additionally ensure
the restore/draft code that recreates state (where drafts are restored) sets
generatedContent from the draft if available so restored drafts pass step4.
- Around line 154-183: The validation/snapshot helpers (getValidation,
getErrors, persistDraft) read stateRef.current but stateRef is only synced in
useEffect, risking stale reads; to fix, ensure the ref is updated immediately
whenever wizardState is updated by setting stateRef.current = newState inside
the state updater functions (e.g., wherever setWizardState or related setters
run) and do the same for draftKeyRef when draftKey is changed, so the ref and
React state remain consistent for immediate reads by
getValidation/getErrors/persistDraft.
- Around line 493-502: discardRecoveredDraft clears the wrong key: it uses the
current draftKey instead of the recovered draft's key, so recoveredDraft can
reappear later; change discardRecoveredDraft to call clearDraftRecord and
setActiveDraftKey using recoveredDraft.draftKey (or guard if recoveredDraft is
null) and only then reset recoveredDraft, draftVersions, and lastSavedAt,
referencing the discardRecoveredDraft function, the recoveredDraft object,
clearDraftRecord, setActiveDraftKey, setRecoveredDraft, setDraftVersions, and
setLastSavedAt to implement the fix.
- Around line 507-519: The resetWizard function currently resets wizardState and
currentStep and clears the draft record/key and localStorage, but it must also
clear draft metadata and sync state so stale UI doesn't remain; update
resetWizard to additionally reset draftVersions, lastSavedAt, syncError, and
recoveredDraft (e.g., call the setters that manage those pieces of state)
alongside the existing clearDraftRecord(draftKey) and setActiveDraftKey(null),
and ensure any in-memory sync flags are reset before removing WIZARD_STATE_KEY
from localStorage.
- Around line 269-275: The setCoverImage callback leaks object URLs because
URL.createObjectURL is used without revoking previous URLs; update setCoverImage
(in the useCreationWizard hook) to revoke the existing prevPreview stored in
wizardState.coverImagePreview via URL.revokeObjectURL before setting a new
preview or clearing it, and ensure the hook also revokes any remaining
coverImagePreview in a cleanup effect (useEffect return) or when resetting state
(wherever resetWizard or similar is implemented) so no object URLs remain after
unmount/reset.
---
Nitpick comments:
In `@components/wizard/step-advanced-params.tsx`:
- Around line 73-111: The Label components for the Art Style and Panel Layout
fields are not associated with their Select controls; update each field to add a
unique id (e.g., artStyle-select and panelLayout-select) on the
Select/SelectTrigger or the underlying input element that the Select library
exposes, and set the corresponding Label's htmlFor to that id so clicking the
label focuses the Select. Apply the same pattern for other fields referenced
(lines ~199-215 and ~231-285), keeping ids unique and preserving existing props
like value (params.artStyle, params.panelLayout) and onValueChange
(handleChange).
In `@components/wizard/step-core-prompt.tsx`:
- Around line 52-57: The error banner rendering for {errors} in the component
(the conditional div in step-core-prompt.tsx) needs ARIA for screen-reader
announcement: update the error div that displays {errors} to include
role="alert" and/or aria-live="assertive" (and optionally aria-atomic="true") so
validation messages are announced; keep the same conditional rendering
referencing errors and only add these attributes to that div.
- Around line 135-140: The blur handler currently reads data.prompt and may use
stale props; update the onBlur handler for the textarea to use the event's
current value (e.g. e.currentTarget.value) instead of data.prompt when computing
trimmed length for analytics: keep calling onBlur?.(), then compute const val =
(e.currentTarget.value || "").trim() and call onPromptAnalytics?.(val.length)
only if val is non-empty. Locate the handler attached to the textarea element
where onBlur, onPromptAnalytics, and data.prompt are referenced and replace the
data.prompt usage with the event value.
In `@components/wizard/step-mode-select.tsx`:
- Around line 68-91: Remove the manual onKeyDown handler from the <button> in
components/wizard/step-mode-select.tsx to avoid double-invoking selection and
analytics (native buttons already handle Enter/Space); keep the existing onClick
that calls onSelect(mode.id) and onAnalytics?.(mode.id). If you need custom
keyboard behavior, implement it only for non-button elements or centralize the
selection logic into a single helper (e.g., callSelect(mode.id) used by onClick)
so selection/analytics invocation is not duplicated elsewhere in this component.
- Line 5: Remove the no-op useEffect import and any empty useEffect calls in
components/wizard/step-mode-select.tsx (they serve no purpose); then tighten the
typing of the onAnalytics prop and any handlers currently using a generic type
to use the StoryMode type (to match onSelect) so onAnalytics receives a
StoryMode parameter; update the prop signature and any internal function
declarations that reference onAnalytics or onSelect accordingly.
In `@components/wizard/step-publish.tsx`:
- Around line 185-242: Prevent changing publish format while a publish is in
progress by disabling the Free/NFT buttons when publishState indicates
publishing; update the button props that use isNft and onPublishFormatChange
(the two <button> elements controlling publish format) to include
disabled={publishState === 'publishing' || publishState === 'inflight'} and
short-circuit onPublishFormatChange to return early if publishState is
publishing. Also gate the Publish button enablement to check actual uploaded
content rather than generatedContent alone by changing its disabled condition to
consider uploadedContent (or files) and use metadataContent = generatedContent
|| corePrompt.prompt where appropriate so the metadata still falls back to
corePrompt.prompt but the Publish action requires uploadedContent to be present.
In `@hooks/use-creation-wizard.ts`:
- Around line 169-181: getValidation currently marks step4 valid only when
state.generatedContent has text, but your snapshot uses content:
state.generatedContent || state.corePrompt.prompt; change step4's check to
validate (s.generatedContent || s.corePrompt.prompt).trim().length > 0 so
pasted-in prompt counts as content. Update the corresponding StepPublish gating
logic (the code around the StepPublish checks referenced in the comment) to use
the same (generatedContent || corePrompt.prompt) trimmed presence check so both
validations are consistent; locate symbols getValidation, stateRef,
WizardValidation, step4, state.generatedContent and state.corePrompt.prompt to
apply the change.
- Around line 298-343: The syncDraftToBackend function can start overlapping
PUTs causing isSyncing flicker; fix by adding an in-flight guard or by aborting
any previous request before starting a new one: create a ref (e.g.,
syncInFlightRef or pendingAbortRef) outside syncDraftToBackend and, at the top
of syncDraftToBackend, either return early if syncInFlightRef.current is true or
call pendingAbortRef.current?.abort() to cancel the previous ctrl; then set
syncInFlightRef.current = true (or store the new AbortController in
pendingAbortRef.current) before fetch and clear/reset that ref in the finally
block (and still clearTimeout tid), ensuring isSyncing state reflects only the
true active request and preventing concurrent PUTs.
In `@hooks/use-wizard-analytics.ts`:
- Around line 77-86: The payload construction allows callers to override
stepLabel because metadata is spread after the hardcoded value; update the merge
so stepLabel from STEP_LABELS[step] is authoritative by merging metadata first
and then setting stepLabel (i.e., ensure metadata is spread before assigning
stepLabel) when building the metadata field for the AnalyticsPayload in the code
that constructs payload (refer to payload, AnalyticsPayload, STEP_LABELS, step,
metadata, sessionIdRef).
- Around line 94-107: Add a non-blocking fetch fallback that uses
fetch('/api/analytics', { method: 'POST', body: JSON.stringify(payload),
headers: {'Content-Type':'application/json'}, keepalive: true }) when
navigator.sendBeacon is not available or fails; update the send block in
hooks/use-wizard-analytics.ts (the navigator.sendBeacon try/catch around payload
creation) to call this fallback (wrapped in try/catch and not awaited) so
analytics are still delivered in environments that disable sendBeacon while
preserving non-blocking behavior.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between e381612 and 9d1e1eba2de7401c946fdc640b8a4e5ac24de10e.
📒 Files selected for processing (12)
app/create/ai-story/page.tsxapp/create/page.tsxcomponents/wizard/creation-wizard.tsxcomponents/wizard/index.tscomponents/wizard/step-advanced-params.tsxcomponents/wizard/step-core-prompt.tsxcomponents/wizard/step-mode-select.tsxcomponents/wizard/step-preview.tsxcomponents/wizard/step-publish.tsxcomponents/wizard/wizard-stepper.tsxhooks/use-creation-wizard.tshooks/use-wizard-analytics.ts
Comment on lines
+208
to
+213
| onSelect={(mode) => { | ||
| wizard.setMode(mode); | ||
| analytics.onModeSelected(mode); | ||
| }} | ||
| onAnalytics={analytics.onModeSelected} | ||
| /> |
Contributor
There was a problem hiding this comment.
Mode selection analytics is emitted twice per click.
Line 210 calls analytics.onModeSelected(mode) inside onSelect, and onAnalytics on Line 212 points to the same handler that the child also invokes.
💡 Suggested fix
{wizard.currentStep === 1 && (
<StepModeSelect
selectedMode={wizard.wizardState.mode}
onSelect={(mode) => {
wizard.setMode(mode);
- analytics.onModeSelected(mode);
}}
onAnalytics={analytics.onModeSelected}
/>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onSelect={(mode) => { | |
| wizard.setMode(mode); | |
| analytics.onModeSelected(mode); | |
| }} | |
| onAnalytics={analytics.onModeSelected} | |
| /> | |
| onSelect={(mode) => { | |
| wizard.setMode(mode); | |
| }} | |
| onAnalytics={analytics.onModeSelected} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/wizard/creation-wizard.tsx` around lines 208 - 213, The
mode-selection handler is invoked twice because analytics.onModeSelected is
called both inside the onSelect callback and again via the onAnalytics prop;
remove the duplicate by having onSelect only call wizard.setMode (remove
analytics.onModeSelected from the onSelect body) and keep the
onAnalytics={analytics.onModeSelected} prop so the child emits analytics once.
Adjust references to wizard.setMode and analytics.onModeSelected accordingly.
| type="button" | ||
| onClick={wizard.goNext} | ||
| disabled={!wizard.canGoNext()} | ||
| className={`comic-button-secondary flex items-center text-sm" ${!wizard.canGoNext() ? 'opacity-50 cursor-not-allowed' : ''}`} |
Contributor
There was a problem hiding this comment.
Malformed className string on Next button.
Line 311 includes text-sm" (extra quote), which breaks that class token.
💡 Suggested fix
- className={`comic-button-secondary flex items-center text-sm" ${!wizard.canGoNext() ? 'opacity-50 cursor-not-allowed' : ''}`}
+ className={`comic-button-secondary flex items-center text-sm ${!wizard.canGoNext() ? 'opacity-50 cursor-not-allowed' : ''}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| className={`comic-button-secondary flex items-center text-sm" ${!wizard.canGoNext() ? 'opacity-50 cursor-not-allowed' : ''}`} | |
| className={`comic-button-secondary flex items-center text-sm ${!wizard.canGoNext() ? 'opacity-50 cursor-not-allowed' : ''}`} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/wizard/creation-wizard.tsx` at line 311, The Next button's JSX has
a malformed className string: remove the stray double-quote inside the template
literal so the token `text-sm` is not broken and ensure the conditional concat
using wizard.canGoNext() remains inside the template literal; update the
className expression (the Next button's className) to produce a single valid
string, e.g., `comic-button-secondary flex items-center text-sm
${!wizard.canGoNext() ? 'opacity-50 cursor-not-allowed' : ''}`.
Comment on lines
+154
to
+183
| const stateRef = useRef(wizardState); | ||
| const latestSigRef = useRef(''); | ||
| const draftKeyRef = useRef(draftKey); | ||
|
|
||
| useEffect(() => { | ||
| stateRef.current = wizardState; | ||
| }, [wizardState]); | ||
| useEffect(() => { | ||
| draftKeyRef.current = draftKey; | ||
| }, [draftKey]); | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
| // Validation | ||
| // ------------------------------------------------------------------------- | ||
|
|
||
| const getValidation = useCallback((): WizardValidation => { | ||
| const s = stateRef.current; | ||
| return { | ||
| step1: s.mode !== null, | ||
| step2: | ||
| s.corePrompt.title.trim().length > 0 && | ||
| s.corePrompt.genre.trim().length > 0 && | ||
| s.corePrompt.prompt.trim().length > 0, | ||
| step3: true, // all advanced params are optional | ||
| step4: | ||
| s.generatedContent !== null && s.generatedContent.trim().length > 0, | ||
| step5: true, | ||
| }; | ||
| }, []); | ||
|
|
Contributor
There was a problem hiding this comment.
Potential stale-state bug: stateRef is only synced in useEffect, but validation/snapshots read stateRef.current.
There’s a window where wizardState has updated but stateRef.current hasn’t, which can cause getValidation(), getErrors(), or persistDraft() to act on stale data in edge cases (fast interactions / back-to-back events).
One robust pattern: sync ref inside the state updater
- const [wizardState, setWizardState] = useState<WizardState>(() => {
+ const [wizardState, setWizardState] = useState<WizardState>(() => {
const base = { ...INITIAL_STATE };
...
return base;
});
- useEffect(() => {
- stateRef.current = wizardState;
- }, [wizardState]);
+ const setWizardStateAndRef = useCallback(
+ (updater: (prev: WizardState) => WizardState) => {
+ setWizardState((prev) => {
+ const next = updater(prev);
+ stateRef.current = next;
+ return next;
+ });
+ },
+ []
+ );
- const setMode = useCallback((mode: StoryMode) => {
- setWizardState((prev) => ({ ...prev, mode }));
- }, []);
+ const setMode = useCallback(
+ (mode: StoryMode) => setWizardStateAndRef((prev) => ({ ...prev, mode })),
+ [setWizardStateAndRef]
+ );(Apply the same pattern to other setters.)
Also applies to: 247-279, 346-394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-creation-wizard.ts` around lines 154 - 183, The validation/snapshot
helpers (getValidation, getErrors, persistDraft) read stateRef.current but
stateRef is only synced in useEffect, risking stale reads; to fix, ensure the
ref is updated immediately whenever wizardState is updated by setting
stateRef.current = newState inside the state updater functions (e.g., wherever
setWizardState or related setters run) and do the same for draftKeyRef when
draftKey is changed, so the ref and React state remain consistent for immediate
reads by getValidation/getErrors/persistDraft.
Comment on lines
+169
to
+201
| const getValidation = useCallback((): WizardValidation => { | ||
| const s = stateRef.current; | ||
| return { | ||
| step1: s.mode !== null, | ||
| step2: | ||
| s.corePrompt.title.trim().length > 0 && | ||
| s.corePrompt.genre.trim().length > 0 && | ||
| s.corePrompt.prompt.trim().length > 0, | ||
| step3: true, // all advanced params are optional | ||
| step4: | ||
| s.generatedContent !== null && s.generatedContent.trim().length > 0, | ||
| step5: true, | ||
| }; | ||
| }, []); | ||
|
|
||
| const getErrors = useCallback((): WizardErrors => { | ||
| const s = stateRef.current; | ||
| return { | ||
| step1: s.mode === null ? 'Please choose a mode to continue.' : null, | ||
| step2: !s.corePrompt.title.trim() | ||
| ? 'Title is required.' | ||
| : !s.corePrompt.genre.trim() | ||
| ? 'Select a genre.' | ||
| : !s.corePrompt.prompt.trim() | ||
| ? 'Enter a prompt or story content.' | ||
| : null, | ||
| step3: null, | ||
| step4: !s.generatedContent?.trim() | ||
| ? 'Generate or write your story content first.' | ||
| : null, | ||
| step5: null, | ||
| }; | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Restore flow likely blocks progression: restored draft sets corePrompt.prompt but not generatedContent (Step 4 requires generatedContent).
Given getValidation().step4 checks generatedContent, a restored draft with content will still fail Step 4 unless generatedContent is also set (or Step 4 validation accepts corePrompt.prompt as content).
Proposed diff (restore sets generatedContent too)
const restoreRecoveredDraft = useCallback(() => {
if (!recoveredDraft) return;
const c = recoveredDraft.current;
setWizardState((prev) => ({
...prev,
corePrompt: {
title: c.title,
description: c.description,
genre: c.genre,
prompt: c.content,
},
+ generatedContent: c.content,
}));Also applies to: 476-491
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-creation-wizard.ts` around lines 169 - 201, The restore flow can
leave corePrompt.prompt populated but generatedContent null, causing
getValidation().step4 and getErrors().step4 to block progression; update the
logic so step4 treats either stateRef.current.generatedContent (non-empty) OR
stateRef.current.corePrompt.prompt (non-empty) as valid, and mirror that change
in getErrors to return null when either field has content; additionally ensure
the restore/draft code that recreates state (where drafts are restored) sets
generatedContent from the draft if available so restored drafts pass step4.
Comment on lines
+269
to
+275
| const setCoverImage = useCallback((file: File | null) => { | ||
| setWizardState((prev) => ({ | ||
| ...prev, | ||
| coverImageFile: file, | ||
| coverImagePreview: file ? URL.createObjectURL(file) : null, | ||
| })); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Memory leak: URL.createObjectURL is never revoked.
If users change cover images multiple times, object URLs will accumulate until refresh. Revoke the previous URL when replacing/clearing and on reset/unmount.
Proposed diff (minimal)
+ const coverPreviewUrlRef = useRef<string | null>(null);
const setCoverImage = useCallback((file: File | null) => {
setWizardState((prev) => ({
...prev,
coverImageFile: file,
- coverImagePreview: file ? URL.createObjectURL(file) : null,
+ coverImagePreview: (() => {
+ if (coverPreviewUrlRef.current) {
+ URL.revokeObjectURL(coverPreviewUrlRef.current);
+ coverPreviewUrlRef.current = null;
+ }
+ if (!file) return null;
+ const url = URL.createObjectURL(file);
+ coverPreviewUrlRef.current = url;
+ return url;
+ })(),
}));
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const setCoverImage = useCallback((file: File | null) => { | |
| setWizardState((prev) => ({ | |
| ...prev, | |
| coverImageFile: file, | |
| coverImagePreview: file ? URL.createObjectURL(file) : null, | |
| })); | |
| }, []); | |
| const coverPreviewUrlRef = useRef<string | null>(null); | |
| const setCoverImage = useCallback((file: File | null) => { | |
| setWizardState((prev) => ({ | |
| ...prev, | |
| coverImageFile: file, | |
| coverImagePreview: (() => { | |
| if (coverPreviewUrlRef.current) { | |
| URL.revokeObjectURL(coverPreviewUrlRef.current); | |
| coverPreviewUrlRef.current = null; | |
| } | |
| if (!file) return null; | |
| const url = URL.createObjectURL(file); | |
| coverPreviewUrlRef.current = url; | |
| return url; | |
| })(), | |
| })); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-creation-wizard.ts` around lines 269 - 275, The setCoverImage
callback leaks object URLs because URL.createObjectURL is used without revoking
previous URLs; update setCoverImage (in the useCreationWizard hook) to revoke
the existing prevPreview stored in wizardState.coverImagePreview via
URL.revokeObjectURL before setting a new preview or clearing it, and ensure the
hook also revokes any remaining coverImagePreview in a cleanup effect (useEffect
return) or when resetting state (wherever resetWizard or similar is implemented)
so no object URLs remain after unmount/reset.
Comment on lines
+493
to
+502
| const discardRecoveredDraft = useCallback(() => { | ||
| if (draftKey) { | ||
| clearDraftRecord(draftKey); | ||
| setActiveDraftKey(null); | ||
| } | ||
| setRecoveredDraft(null); | ||
| setDraftVersions([]); | ||
| setLastSavedAt(null); | ||
| }, [draftKey]); | ||
|
|
Contributor
There was a problem hiding this comment.
Discard flow clears the wrong draft key when recoveredDraft is not the current draft.
discardRecoveredDraft() clears draftKey (freshly created) rather than recoveredDraft.draftKey, so the same recovered draft can reappear on next load.
Proposed diff
const discardRecoveredDraft = useCallback(() => {
- if (draftKey) {
- clearDraftRecord(draftKey);
- setActiveDraftKey(null);
- }
+ if (recoveredDraft) {
+ clearDraftRecord(recoveredDraft.draftKey);
+ } else if (draftKey) {
+ clearDraftRecord(draftKey);
+ }
+ setActiveDraftKey(null);
setRecoveredDraft(null);
setDraftVersions([]);
setLastSavedAt(null);
- }, [draftKey]);
+ }, [draftKey, recoveredDraft]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const discardRecoveredDraft = useCallback(() => { | |
| if (draftKey) { | |
| clearDraftRecord(draftKey); | |
| setActiveDraftKey(null); | |
| } | |
| setRecoveredDraft(null); | |
| setDraftVersions([]); | |
| setLastSavedAt(null); | |
| }, [draftKey]); | |
| const discardRecoveredDraft = useCallback(() => { | |
| if (recoveredDraft) { | |
| clearDraftRecord(recoveredDraft.draftKey); | |
| } else if (draftKey) { | |
| clearDraftRecord(draftKey); | |
| } | |
| setActiveDraftKey(null); | |
| setRecoveredDraft(null); | |
| setDraftVersions([]); | |
| setLastSavedAt(null); | |
| }, [draftKey, recoveredDraft]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-creation-wizard.ts` around lines 493 - 502, discardRecoveredDraft
clears the wrong key: it uses the current draftKey instead of the recovered
draft's key, so recoveredDraft can reappear later; change discardRecoveredDraft
to call clearDraftRecord and setActiveDraftKey using recoveredDraft.draftKey (or
guard if recoveredDraft is null) and only then reset recoveredDraft,
draftVersions, and lastSavedAt, referencing the discardRecoveredDraft function,
the recoveredDraft object, clearDraftRecord, setActiveDraftKey,
setRecoveredDraft, setDraftVersions, and setLastSavedAt to implement the fix.
Comment on lines
+507
to
+519
| const resetWizard = useCallback(() => { | ||
| setWizardState(INITIAL_STATE); | ||
| setCurrentStep(1); | ||
| if (draftKey) { | ||
| clearDraftRecord(draftKey); | ||
| setActiveDraftKey(null); | ||
| } | ||
| try { | ||
| localStorage.removeItem(WIZARD_STATE_KEY); | ||
| } catch { | ||
| // Ignore localStorage errors during cleanup | ||
| } | ||
| }, [draftKey]); |
Contributor
There was a problem hiding this comment.
resetWizard() should also clear draft metadata + sync state.
Right now it resets wizardState/currentStep but leaves draftVersions, lastSavedAt, syncError, and recoveredDraft intact, which can keep stale UI around after “reset”.
Proposed diff
const resetWizard = useCallback(() => {
setWizardState(INITIAL_STATE);
setCurrentStep(1);
+ setDraftVersions([]);
+ setLastSavedAt(null);
+ setSyncError(null);
+ setRecoveredDraft(null);
if (draftKey) {
clearDraftRecord(draftKey);
setActiveDraftKey(null);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-creation-wizard.ts` around lines 507 - 519, The resetWizard
function currently resets wizardState and currentStep and clears the draft
record/key and localStorage, but it must also clear draft metadata and sync
state so stale UI doesn't remain; update resetWizard to additionally reset
draftVersions, lastSavedAt, syncError, and recoveredDraft (e.g., call the
setters that manage those pieces of state) alongside the existing
clearDraftRecord(draftKey) and setActiveDraftKey(null), and ensure any in-memory
sync flags are reset before removing WIZARD_STATE_KEY from localStorage.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
suhaib-ilahi
force-pushed
the
fix#321-redesign-flow-storycreation
branch
from
9d1e1eb to
cd567f8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/auth/admin-status/route.ts`:
- Around line 7-15: The GET handler's admin check (in function GET) currently
trusts client-writable cookies adminSessionActive and adminSessionToken; replace
this with a server-validated session lookup: accept the adminSessionToken cookie
value but verify it against your session store or token verifier (e.g.,
validateSessionToken(sessionToken) or lookupAdminSession(sessionToken)) and
derive isAdmin from that verification instead of mere presence; also change the
login/middleware writers to set the token cookie as HttpOnly, Secure and
SameSite (set via server response cookies, not document.cookie in the client),
and update any helpers that read cookies to expect only server-set httpOnly
tokens so client-side spoofing cannot mark isAdmin true.
In `@app/create/ai-story/page.tsx`:
- Around line 58-67: The effect in useEffect that builds storyData and calls
localStorage.setItem('storyCreationData', ...) overwrites any existing selection
context; modify it to first read and parse existing
localStorage.getItem('storyCreationData') and only write the new storyData if
the page was opened with non-default selections (e.g., source is set or
format/genre differ from their default values) or if there is no existing saved
data; otherwise skip the setItem to avoid clobbering other writers. Reference:
update the useEffect block that constructs storyData and the
localStorage.setItem call, and use the variables format, genre, source and the
key 'storyCreationData' to decide whether to persist.
In `@app/create/page.tsx`:
- Around line 12-18: Remove the server-side Supabase import and unused
instantiation from the client component: delete the import of createClient from
'@/lib/supabase/server' and remove the supabase = createClient() line inside
CreateStoryPage (remove references to createClient and the supabase variable),
ensuring the component no longer relies on the server-only
cookies()/next/headers API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb97eb06-a6c7-4ea8-9545-42b7d20cac84
📥 Commits
Reviewing files that changed from the base of the PR and between 9d1e1eba2de7401c946fdc640b8a4e5ac24de10e and cd567f8.
📒 Files selected for processing (3)
app/api/auth/admin-status/route.tsapp/create/ai-story/page.tsxapp/create/page.tsx
Comment on lines
+7
to
+15
| * Relies on the httpOnly-style cookies set by the admin login flow | ||
| * and the Next.js middleware – never trusts client-set localStorage. | ||
| */ | ||
| export async function GET(request: NextRequest) { | ||
| const cookies = request.cookies; | ||
| const sessionActive = cookies.get('adminSessionActive')?.value === 'true'; | ||
| const sessionToken = cookies.get('adminSessionToken')?.value; | ||
|
|
||
| const isAdmin = sessionActive && !!sessionToken; |
Contributor
There was a problem hiding this comment.
Cookie-presence auth check is forgeable (admin spoofing risk).
At Line [12]-[15], isAdmin is derived from client-writable cookie presence/value only. Given app/admin/login/page.tsx (Line 159-160) sets these via document.cookie, an attacker can set both cookies manually and get isAdmin: true. The Line [7]-[8] comment about “httpOnly-style cookies” is also inconsistent with current setters.
🔐 Suggested direction (server-validated session token)
import { NextRequest, NextResponse } from 'next/server';
+import { verifyAdminSessionToken } from '@/lib/auth/admin-session';
export async function GET(request: NextRequest) {
- const cookies = request.cookies;
- const sessionActive = cookies.get('adminSessionActive')?.value === 'true';
- const sessionToken = cookies.get('adminSessionToken')?.value;
-
- const isAdmin = sessionActive && !!sessionToken;
+ const sessionToken = request.cookies.get('adminSessionToken')?.value;
+ const isAdmin = sessionToken
+ ? await verifyAdminSessionToken(sessionToken) // signature + expiry + subject/role checks
+ : false;
return NextResponse.json({ isAdmin });
}Also ensure the login/middleware cookie writers set httpOnly: true, secure: true, and sameSite: 'strict'|'lax' from server headers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/auth/admin-status/route.ts` around lines 7 - 15, The GET handler's
admin check (in function GET) currently trusts client-writable cookies
adminSessionActive and adminSessionToken; replace this with a server-validated
session lookup: accept the adminSessionToken cookie value but verify it against
your session store or token verifier (e.g., validateSessionToken(sessionToken)
or lookupAdminSession(sessionToken)) and derive isAdmin from that verification
instead of mere presence; also change the login/middleware writers to set the
token cookie as HttpOnly, Secure and SameSite (set via server response cookies,
not document.cookie in the client), and update any helpers that read cookies to
expect only server-set httpOnly tokens so client-side spoofing cannot mark
isAdmin true.
Comment on lines
58
to
67
| useEffect(() => { | ||
| try { | ||
| const storyData = { type: 'ai', format, genre, redirectToCreate: !!source, timestamp: Date.now() }; | ||
| const storyData = { | ||
| type: 'ai', | ||
| format, | ||
| genre, | ||
| redirectToCreate: !!source, | ||
| timestamp: Date.now(), | ||
| }; | ||
| localStorage.setItem('storyCreationData', JSON.stringify(storyData)); |
Contributor
There was a problem hiding this comment.
Prevent default-state overwrites of storyCreationData.
This effect always writes storyCreationData, which can clobber previously saved selection context from other writers of the same key (see components/create-story-dialog.tsx Line 96-115). Guard writes when the page is opened with only defaults.
💡 Suggested fix
useEffect(() => {
try {
+ const existing = localStorage.getItem('storyCreationData');
+ const isDefaultEntry = !source && genre === 'fantasy' && format === 'free';
+ if (existing && isDefaultEntry) return;
+
const storyData = {
type: 'ai',
format,
genre,
redirectToCreate: !!source,
timestamp: Date.now(),
};
localStorage.setItem('storyCreationData', JSON.stringify(storyData));
} catch (error) {
console.error('Error setting up story creation data:', error);
}
}, [source, genre, format]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| try { | |
| const storyData = { type: 'ai', format, genre, redirectToCreate: !!source, timestamp: Date.now() }; | |
| const storyData = { | |
| type: 'ai', | |
| format, | |
| genre, | |
| redirectToCreate: !!source, | |
| timestamp: Date.now(), | |
| }; | |
| localStorage.setItem('storyCreationData', JSON.stringify(storyData)); | |
| useEffect(() => { | |
| try { | |
| const existing = localStorage.getItem('storyCreationData'); | |
| const isDefaultEntry = !source && genre === 'fantasy' && format === 'free'; | |
| if (existing && isDefaultEntry) return; | |
| const storyData = { | |
| type: 'ai', | |
| format, | |
| genre, | |
| redirectToCreate: !!source, | |
| timestamp: Date.now(), | |
| }; | |
| localStorage.setItem('storyCreationData', JSON.stringify(storyData)); | |
| } catch (error) { | |
| console.error('Error setting up story creation data:', error); | |
| } | |
| }, [source, genre, format]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/create/ai-story/page.tsx` around lines 58 - 67, The effect in useEffect
that builds storyData and calls localStorage.setItem('storyCreationData', ...)
overwrites any existing selection context; modify it to first read and parse
existing localStorage.getItem('storyCreationData') and only write the new
storyData if the page was opened with non-default selections (e.g., source is
set or format/genre differ from their default values) or if there is no existing
saved data; otherwise skip the setItem to avoid clobbering other writers.
Reference: update the useEffect block that constructs storyData and the
localStorage.setItem call, and use the variables format, genre, source and the
key 'storyCreationData' to decide whether to persist.
Comment on lines
+12
to
18
| import { createClient } from '@/lib/supabase/server'; | ||
|
|
||
| export default function CreateStoryPage() { | ||
| const router = useRouter(); | ||
| const { account } = useWeb3(); | ||
| const { toast } = useToast(); | ||
| const supabase = createClient(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the create page
find . -name "create" -type d | head -5Repository: IndieHub25/GroqTales
Length of output: 76
🏁 Script executed:
# Locate the supabase server helper and create page
fd -t f "page.tsx" | grep -E "create.*page"Repository: IndieHub25/GroqTales
Length of output: 112
🏁 Script executed:
# Find supabase server helper
fd -t f "server\.(ts|tsx|js|jsx)$" | grep supabaseRepository: IndieHub25/GroqTales
Length of output: 86
🏁 Script executed:
cat -n app/create/page.tsxRepository: IndieHub25/GroqTales
Length of output: 3723
🏁 Script executed:
cat -n lib/supabase/server.tsRepository: IndieHub25/GroqTales
Length of output: 1751
Remove unused server-side Supabase import from this client component.
Line 12 imports @/lib/supabase/server, which uses cookies() from next/headers—a server-only API that cannot execute in client components. Line 18 instantiates it but the variable is never used. Remove both the import and instantiation to prevent runtime errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/create/page.tsx` around lines 12 - 18, Remove the server-side Supabase
import and unused instantiation from the client component: delete the import of
createClient from '@/lib/supabase/server' and remove the supabase =
createClient() line inside CreateStoryPage (remove references to createClient
and the supabase variable), ensuring the component no longer relies on the
server-only cookies()/next/headers API.
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
2 participants
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.
Description
Issue Reference (e.g. Fixes #123): Fixes #321
Summary of Changes (high-level):
Context / Motivation:
bg-card,text-foreground,border-foreground,hsl(var(--foreground))) to ensure correct rendering in both light and dark mode — no hardcoded colors.useCreationWizardhook withlocalStoragepersistence.Type of Change
Select the relevant categories:
Technical Checklist
Tick everything that applies. Leave non‑applicable items unchecked.
AI / Application Logic
GROQ_API_KEY.Web3 / Smart Contracts
Frontend / UX / Accessibility
aria-*, alt text) and semantic HTML for new UI.Backend / Database
Security & Privacy
.envfiles are committed.Code Quality
npm run lint(or equivalent) and resolved reported issues.anyby default.Testing Evidence
Describe how you tested these changes. Include commands and logs where possible.
Manual test steps for UI flows:
/create→ verify Mode Select step renders with Story/Comic cardsVisual Proof (for UI / UX changes)
Uploading Screen Recording 2026-03-02 090540.mp4…
Contributor Status
Tick all that apply to you for this PR:
Review & Impact
Breaking Changes
Dependencies
Backward Compatibility / Migrations
Final Acknowledgements (Mandatory or will be marked invalid)
You must check all of the following before requesting review. These are required:
Files Changed (12 files, +2,549 / −1,492)
<CreationWizard />Summary by CodeRabbit
New Features
Improvements