feat: add remix cli preview and move routes to follow remix conventions#135
Merged
Conversation
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
|
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR adds agent skills and documentation for Remix layout/UI, a TypeScript bootstrap script to generate a Remix app skeleton, introduces oxlint plugins and CI/editor hooks, migrates many imports to Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,230,255,0.5)
participant Client
participant Router
participant LoginController
participant SessionStore
participant DB
end
Client->>Router: POST /auth/login (credentials)
Router->>LoginController: route -> actions.action(context)
LoginController->>DB: verifyCredentials(email, password)
DB-->>LoginController: user|null
alt user found
LoginController->>SessionStore: completeAuth(session, { userId })
SessionStore-->>LoginController: session updated
LoginController-->>Router: redirect to post-auth URL (with session cookie)
Router-->>Client: 302 redirect (Set-Cookie)
else auth failed
LoginController->>SessionStore: flash("Invalid email or password")
LoginController-->>Router: redirect back to login (with flashed error)
Router-->>Client: 302 redirect (Set-Cookie)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Pull request overview
This PR introduces Remix CLI preview support and reorganizes application controllers/routes to better align with Remix project layout conventions, alongside adding agent “skills” documentation and a bootstrap script.
Changes:
- Added
@remix-run/clifrom a preview GitHub ref and updated workspace/lockfiles. - Moved/rewired controllers to a convention-based
app/controllers/**structure (including nested auth controllers) and updated router imports. - Added
vp stagedintegration (Vite config + pre-commit hook) and expanded.agents/skills/**docs/scripts.
Reviewed changes
Copilot reviewed 31 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Adds vp staged config (but needs fmt override paths updated for moved files). |
| tsconfig.json | Adds Node + Vite client ambient types. |
| pnpm-workspace.yaml | Adds catalog entry for preview @remix-run/cli. |
| pnpm-lock.yaml | Locks preview CLI tarball and related snapshot entries. |
| package.json | Adds @remix-run/cli dependency via catalog. |
| app/router.ts | Updates controller import paths to new app/controllers/** layout and maps health route. |
| app/controllers/home/page.tsx | Updates relative imports after controller move. |
| app/controllers/home/local-schema.ts | Adds TS suppression comments for indexed ~run calls. |
| app/controllers/home/local-form-schema.ts | Adds a local Standard Schema-compatible FormData/URLSearchParams parser/validator. |
| app/controllers/home/controller.tsx | Updates imports to match new controller folder structure. |
| app/controllers/home/controller.test.ts | Updates import paths after move (but mocks need specifier fixes). |
| app/controllers/history/not-found-page.tsx | Updates relative imports after controller move. |
| app/controllers/history/history-page.tsx | Updates relative imports after controller move. |
| app/controllers/history/game.tsx | Updates relative imports after controller move. |
| app/controllers/history/controller.tsx | Updates relative imports after controller move. |
| app/controllers/health.tsx | Adds a new health endpoint action (currently builds fetch URL from Host headers). |
| app/controllers/auth/controller.tsx | Replaces monolithic auth controller with nested sub-controllers (has unused imports). |
| app/controllers/auth/controller.test.ts | Updates import paths after move (but mocks need specifier fixes). |
| app/controllers/auth/login/controller.tsx | New nested login controller. |
| app/controllers/auth/register/controller.tsx | New nested register controller. |
| app/controllers/auth/forgot-password/controller.tsx | New nested forgot-password controller. |
| app/controllers/auth/reset-password/controller.tsx | New nested reset-password controller. |
| app/auth/controller.tsx | Removes the previous monolithic auth controller implementation. |
| AGENTS.md | Adds CI integration guidance for voidzero-dev/setup-vp. |
| .vite-hooks/pre-commit | Adds hook to run vp staged. |
| .agents/skills/remix-ui/SKILL.md | Introduces “remix-ui” agent skill documentation. |
| .agents/skills/remix-ui/references/testing-patterns.md | Adds UI testing reference material. |
| .agents/skills/remix-ui/references/mixins-styling-events.md | Adds mixins/events/styling reference material. |
| .agents/skills/remix-ui/references/hydration-frames-navigation.md | Adds hydration/frames/navigation reference material. |
| .agents/skills/remix-ui/references/create-mixins.md | Adds mixin authoring reference material. |
| .agents/skills/remix-ui/references/component-model.md | Adds component model reference material. |
| .agents/skills/remix-ui/references/animate-elements.md | Adds animation reference material. |
| .agents/skills/remix-project-layout/tsconfig.json | Adds TS config for the bootstrap script/tooling. |
| .agents/skills/remix-project-layout/SKILL.md | Introduces “remix-project-layout” agent skill documentation. |
| .agents/skills/remix-project-layout/scripts/bootstrap_remix_application.ts | Adds a script to scaffold a convention-based Remix app layout. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
app/controllers/home/controller.test.ts:11
- These mocks are targeting "./models/game.ts" (and similarly "./models/user.ts"), but the code under test imports from "../../models/...". Because the specifiers don’t match, the mocks won’t apply. Update the vi.mock specifiers to match the actual import paths used by app/controllers/home/controller.tsx.
app/controllers/auth/controller.test.ts:8 - This test mocks "./models/user.ts", but the auth/login/register controllers import the user model via different specifiers (e.g., "../../models/user.ts" or "../../../models/user"). With mismatched specifiers, the mock won’t intercept the real module. Adjust the vi.mock path(s) to match the module specifier actually imported by the code under test.
![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: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/remix-ui/references/animate-elements.md:
- Line 1: The top-level heading "## Animating Elements (`remix/component`)" must
be changed to an H1 to satisfy markdownlint MD041; replace the leading "##" with
a single "#" so the line reads "# Animating Elements (`remix/component`)" and
ensure there are no preceding blank lines before this heading.
In @.agents/skills/remix-ui/references/component-model.md:
- Line 1: The markdown heading "Component Model" is currently using a level-2
heading (`##`), violating MD041; change the opening heading token for "Component
Model" from `##` to a level-1 heading `#` so the file's top-level heading is H1
and satisfies the lint rule (MD041).
In @.agents/skills/remix-ui/references/create-mixins.md:
- Line 1: The top-level heading currently uses H2; update the opening line in
create-mixins.md from "## Creating Mixins (`remix/component`)" to a level-1
heading by changing it to "# Creating Mixins (`remix/component`)" so the file
satisfies MD041 (top-level heading required).
In @.agents/skills/remix-ui/references/hydration-frames-navigation.md:
- Line 1: The file's top-level heading uses "## Hydration" which violates MD041;
update the initial heading to use a single H1 by replacing the leading "##" with
"#" so the first line reads "# Hydration" (ensure the heading text "Hydration"
remains unchanged and only the heading level is adjusted).
In @.agents/skills/remix-ui/references/mixins-styling-events.md:
- Line 1: Replace the first-line level-2 heading with a top-level H1 by changing
the leading "##" to a single "#" for "Host Elements" (i.e., make the first line
"# Host Elements") and ensure it is the very first line in the file with no
preceding blank lines so MD041 (first-line-heading) is satisfied.
In @.agents/skills/remix-ui/references/testing-patterns.md:
- Line 1: The file's top-level heading uses "## Testing" which triggers MD041
(first heading should be H1); change the first-line heading to a single hash
(e.g., "# Testing") so the document begins with an H1, leaving the rest of the
content unchanged and ensuring the heading text remains "Testing".
In @.vite-hooks/pre-commit:
- Line 1: The pre-commit hook file .vite-hooks/pre-commit contains the correct
command (vp staged) but Git won't run it by default; either set core.hooksPath
to ".vite-hooks" in vite.config.ts or ensure the repo setup docs/Review
Checklist instruct developers to run the vp config command after cloning; modify
vite.config.ts to include core.hooksPath: ".vite-hooks" (or add a clear setup
step telling devs to run vp config) so hooks are wired automatically for new
clones.
In `@app/controllers/auth/controller.test.ts`:
- Around line 6-7: The mock specifier passed to vi.mock does not match the
actual module path used by importActual, so update the vi.mock call to target
the same module string as the importActual call: change the mock target in
vi.mock to "../../models/user.ts" so the mock binds correctly to the module
referenced by importActual (the vi.mock(..., async (importActual) => { let
actual = await importActual<typeof import("../../models/user.ts")>() }) block).
In `@app/controllers/auth/forgot-password/controller.tsx`:
- Around line 46-60: This block is rendering the live reset token into HTML (the
token variable used with routes.auth.resetPassword.index.href), which leaks
reset tokens; remove this UI path so the token is never embedded in a response —
instead return a generic success/notice message to the requester and
generate/send the reset URL server-side via email. Concretely: delete or disable
the conditional that renders the token-based link (the JSX using token and
routes.auth.resetPassword.index.href), ensure the password reset URL is produced
and emailed on the server/controller side only, and leave the page showing a
non-sensitive confirmation message like "If an account with that email exists,
you will receive reset instructions." Ensure no other code paths render the
token variable.
In `@app/controllers/auth/login/controller.tsx`:
- Around line 9-11: The controller currently imports shared schema utilities via
localSchema.parse which couples auth/login to the home controller; extract the
shared validation primitive(s) (e.g., parse) from "../../home/local-schema" into
a neutral module under app/utils/ or app/schemas/ (create an exported function
or re-export for parse), then update the auth/login controller to import parse
from the new module instead of localSchema; ensure any related types or helper
functions used by parse are moved or re-exported so the auth/login controller no
longer depends on the home controller.
In `@app/controllers/auth/register/controller.tsx`:
- Line 15: The Document component's head prop currently sets the page title to
"Login - Remix Wordle"; update the title to reflect registration (e.g.,
"Register - Remix Wordle" or "Sign Up - Remix Wordle") wherever Document is used
with head={<title>Login - Remix Wordle</title>} (both occurrences of that
Document usage), so the page shows the correct register/signup title.
- Line 118: The session is being set with the wrong shape — change the call to
session.set("auth", { auth: user.id }) so it stores { userId: user.id } instead,
because parseAuthSession expects { userId: s.string() } and the auth middleware
reads result.value.userId; update the register controller's session.set to match
the login controller's usage and ensure any downstream code that reads
session.get("auth") continues to expect userId.
In `@app/controllers/auth/reset-password/controller.tsx`:
- Line 18: The page title is incorrect in the reset-password view: update the
<title> passed to the Document component (the JSX instance Document url={url}
head={<title>...>}) to read "Reset Password - Remix Wordle" instead of "Login -
Remix Wordle"; make the same change for the second Document/title occurrence
used later in this file so both instances show the reset-password title.
- Around line 72-77: The reset flow is broken because resetPassword in
app/models/user.ts currently bypasses validation and always returns true, making
the error branch in the controller (reset-password controller using
resetPassword(params.token, password)) unreachable; restore the validation logic
in resetPassword so it verifies the token, expiry, and actually updates the user
password (return true on success, false on failure) or throw a clear error and
have the controller handle it, and update the controller to handle the real
failure case accordingly; also update the page title strings in the
reset-password controller (currently "Login" at the title locations) to "Reset
Password" so the UI reflects the correct page.
In `@app/controllers/home/controller.test.ts`:
- Around line 9-10: The vi.mock specifiers are wrong: change the module
specifiers passed to vi.mock (the "./models/..." strings) to the correct
relative paths used in the importActual generics (../../models/...), so the
mocks target the real model modules; update both vi.mock calls in
controller.test.ts (the ones mocking game and the other model referenced with
importActual) to use "../../models/<module>.ts" to match the importActual type
references.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: c5de30b9-3076-4a7b-9b1a-ea718041b264
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.agents/skills/remix-project-layout/SKILL.md.agents/skills/remix-project-layout/scripts/bootstrap_remix_application.ts.agents/skills/remix-project-layout/tsconfig.json.agents/skills/remix-ui/SKILL.md.agents/skills/remix-ui/references/animate-elements.md.agents/skills/remix-ui/references/component-model.md.agents/skills/remix-ui/references/create-mixins.md.agents/skills/remix-ui/references/hydration-frames-navigation.md.agents/skills/remix-ui/references/mixins-styling-events.md.agents/skills/remix-ui/references/testing-patterns.md.vite-hooks/pre-commitAGENTS.mdapp/auth/controller.tsxapp/controllers/auth/controller.test.tsapp/controllers/auth/controller.tsxapp/controllers/auth/forgot-password/controller.tsxapp/controllers/auth/login/controller.tsxapp/controllers/auth/register/controller.tsxapp/controllers/auth/reset-password/controller.tsxapp/controllers/health.tsxapp/controllers/history/controller.tsxapp/controllers/history/game.tsxapp/controllers/history/history-page.tsxapp/controllers/history/not-found-page.tsxapp/controllers/home/controller.test.tsapp/controllers/home/controller.tsxapp/controllers/home/local-form-schema.tsapp/controllers/home/local-schema.tsapp/controllers/home/page.tsxapp/middleware/auth.tsapp/router.tspackage.jsonpnpm-workspace.yamltsconfig.jsonvite.config.ts
💤 Files with no reviewable changes (1)
- app/auth/controller.tsx
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/models/user.ts (1)
55-67:⚠️ Potential issue | 🔴 CriticalCritical: Missing
awaiton async function call.Line 56 calls the async function
getUserByEmail(email)withoutawait, andcreatePasswordResetTokenis not declared asasync. This assigns aPromise<User | null>touserinstead of the resolved value. The subsequent check on line 57 (if (!user)) will always be falsy because Promises are truthy objects, breaking the intended logic.🐛 Proposed fix
-export function createPasswordResetToken(email: string): string | undefined { - let user = getUserByEmail(email) +export async function createPasswordResetToken(email: string): Promise<string | undefined> { + let user = await getUserByEmail(email) if (!user) return undefined let token = Math.random().toString(36).substring(2, 15)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/user.ts` around lines 55 - 67, The createPasswordResetToken function calls the async getUserByEmail(email) without awaiting it, so change createPasswordResetToken to an async function, await getUserByEmail(email), and update its return type to Promise<string | undefined>; inside the async function use "const user = await getUserByEmail(email)" and keep the existing null check and token generation logic (ensure callers of createPasswordResetToken are updated to handle a Promise).
♻️ Duplicate comments (9)
app/controllers/auth/controller.test.ts (1)
6-7:⚠️ Potential issue | 🔴 CriticalFix
vi.mockspecifier mismatch (Line 6).
vi.mock("./models/user.ts")will not mock the module referenced as"../../models/user.ts"at Line 7. Use the same specifier string in both places.🔧 Proposed fix
-vi.mock("./models/user.ts", async (importActual) => { +vi.mock("../../models/user.ts", async (importActual) => { let actual = await importActual<typeof import("../../models/user.ts")>()#!/bin/bash set -euo pipefail # Verifies that each vi.mock("...") target matches the corresponding importActual<typeof import("...")>() path. python - <<'PY' import re, pathlib p = pathlib.Path("app/controllers/auth/controller.test.ts") t = p.read_text() pattern = re.compile(r'vi\.mock\("([^"]+)"[\s\S]*?importActual<[^>]*import\("([^"]+)"\)', re.M) found = False for i, (mock_path, actual_path) in enumerate(pattern.findall(t), 1): found = True status = "OK" if mock_path == actual_path else "MISMATCH" print(f"{i}. mock={mock_path} actual={actual_path} => {status}") if not found: print("No vi.mock/importActual pairs found.") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/controller.test.ts` around lines 6 - 7, The vi.mock specifier string doesn't match the importActual specifier causing the mock to not apply; update the vi.mock call so its first argument string equals the string used inside importActual<typeof import("...")>(), e.g. change vi.mock("./models/user.ts", ...) to use the exact same module specifier used in importActual (or vice versa) so the mock pair (vi.mock and importActual) reference the identical module name; ensure this consistency for the vi.mock(...) and importActual<typeof import("...")>() pair in the test file.app/controllers/home/controller.test.ts (1)
10-11:⚠️ Potential issue | 🔴 CriticalFix both
vi.mocktarget specifiers to matchimportActualpaths.At Lines 10-11 and 38-39, the mock target strings (
"./models/...") do not match the typed actual import paths ("../../models/..."), so mocks can fail to apply.🔧 Proposed fix
-vi.mock("./models/game.ts", async (importActual) => { +vi.mock("../../models/game.ts", async (importActual) => { let actual = await importActual<typeof import("../../models/game.ts")>()-vi.mock("./models/user.ts", async (importActual) => { +vi.mock("../../models/user.ts", async (importActual) => { let actual = await importActual<typeof import("../../models/user.ts")>()#!/bin/bash set -euo pipefail # Confirms each vi.mock target path matches its importActual type-import path. python - <<'PY' import re, pathlib p = pathlib.Path("app/controllers/home/controller.test.ts") t = p.read_text() pattern = re.compile(r'vi\.mock\("([^"]+)"[\s\S]*?importActual<[^>]*import\("([^"]+)"\)', re.M) pairs = pattern.findall(t) if not pairs: print("No vi.mock/importActual pairs found.") else: for idx, (mock_path, actual_path) in enumerate(pairs, 1): print(f"{idx}. mock={mock_path} actual={actual_path} status={'OK' if mock_path == actual_path else 'MISMATCH'}") PYAlso applies to: 38-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/home/controller.test.ts` around lines 10 - 11, The vi.mock target strings do not match the typed importActual paths, causing mocks to not apply; update each vi.mock call so its first argument exactly matches the path used inside importActual's type-import (change "./models/..." to "../../models/..." to mirror importActual<...import("../../models/xxx")>), ensuring every vi.mock("...") string equals the corresponding importActual<...import("...")> path.app/controllers/auth/forgot-password/controller.tsx (1)
57-71:⚠️ Potential issue | 🔴 CriticalDo not render live password reset tokens in the response HTML.
Line 57-71 exposes a usable reset URL to the requester when a token exists, which leaks sensitive reset credentials.
🔒 Proposed fix
- {token ? ( - <div style="margin-top: 1rem; padding: 1rem; background: `#f8f9fa`; border-radius: 4px;"> - <p style="font-size: 0.9rem;"> - <strong>Demo Mode:</strong> Click the link below to reset your password - </p> - <p style="margin-top: 0.5rem;"> - <a - href={routes.auth.resetPassword.index.href({ token })} - class="btn btn-secondary" - > - Reset Password - </a> - </p> - </div> - ) : null} + {/* Never render reset tokens to clients. Deliver reset URL out-of-band (e.g., email). */}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/forgot-password/controller.tsx` around lines 57 - 71, The current JSX renders a live password reset link when the local variable token is present (token and routes.auth.resetPassword.index.href are used), which leaks credentials; remove the block that outputs the anchor with the actual href and instead replace it with a non-sensitive message (e.g., “If an account exists, a password reset link has been sent to your email”) or display a masked/token-not-shown notice; ensure no code elsewhere in forgot-password controller.tsx returns or injects the raw token into the HTML response.app/controllers/auth/register/controller.tsx (3)
118-118:⚠️ Potential issue | 🔴 CriticalSession data structure mismatch breaks authentication after registration.
The session stores
{ auth: user.id }butparseAuthSessioninapp/utils/auth-session.ts:17-28expects{ userId: string }. This causes newly registered users to fail authentication immediately—the schema validation will returnnullsinceuserIdis missing.The login controller at
app/controllers/auth/login/controller.tsx:30correctly uses{ userId: user.id }.- session.set("auth", { auth: user.id }) + session.set("auth", { userId: user.id })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/register/controller.tsx` at line 118, The session is being set with the wrong shape in session.set("auth", { auth: user.id }) which does not match parseAuthSession (expected { userId: string }); change the session write to use the same property name as login/controller.tsx — call session.set("auth", { userId: user.id }) so parseAuthSession will validate and return the newly registered user; ensure any other places that read or write the "auth" session use the { userId } shape for consistency with parseAuthSession.
15-15:⚠️ Potential issue | 🟡 MinorPage title displays "Login" but this is the registration page.
The title should reflect the registration flow.
- <Document url={url} head={<title>Login - Remix Wordle</title>}> + <Document url={url} head={<title>Register - Remix Wordle</title>}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/register/controller.tsx` at line 15, The Document component on the registration page is using an incorrect title; update the head prop in the Document usage (the line rendering Document with url and head) to use an appropriate registration title such as "Register - Remix Wordle" or "Sign Up - Remix Wordle" so the page title matches the registration flow.
91-91:⚠️ Potential issue | 🟡 MinorSame title issue in the error rendering path.
- <Document url={url} head={<title>Login - Remix Wordle</title>}> + <Document url={url} head={<title>Register - Remix Wordle</title>}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/register/controller.tsx` at line 91, The error rendering path in the register controller uses the wrong page title ("Login - Remix Wordle"); update the Document head title in the error branch from "Login - Remix Wordle" to the correct "Register - Remix Wordle" (look for the Document component instance with Document url={url} head={<title>Login - Remix Wordle</title>}), and verify any other titles in this file or the register error rendering path are similarly corrected to "Register - Remix Wordle".app/controllers/auth/reset-password/controller.tsx (3)
87-87:⚠️ Potential issue | 🟡 MinorSame title issue on the success page.
- <Document url={url} head={<title>Login - Remix Wordle</title>}> + <Document url={url} head={<title>Reset Password - Remix Wordle</title>}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/reset-password/controller.tsx` at line 87, The success page currently uses the same Document head title ("Login - Remix Wordle") as the login page; update the Document component used in the reset-password success render so its head prop contains an appropriate title like "Password Reset Success - Remix Wordle" (or "Reset Password - Remix Wordle") instead of "Login - Remix Wordle"—look for the Document component and its head/title usage in the reset-password controller (the line with Document url={url} head={<title>...</title>}) and change the title string accordingly.
20-20:⚠️ Potential issue | 🟡 MinorPage title displays "Login" but this is the reset password page.
- <Document url={url} head={<title>Login - Remix Wordle</title>}> + <Document url={url} head={<title>Reset Password - Remix Wordle</title>}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/reset-password/controller.tsx` at line 20, The page title in the Document component within reset-password controller.tsx is incorrectly set to "Login"; update the head <title> used in Document (the JSX element rendering title inside Document url={url}) to a correct label such as "Reset Password - Remix Wordle" (or similar) so the browser tab and accessibility metadata reflect this is the reset password page.
79-84:⚠️ Potential issue | 🔴 CriticalDead code:
resetPasswordalways returnstrue, making the error branch unreachable.Per
app/models/user.ts:69-82,resetPasswordhas all validation logic commented out and unconditionally returnstrue. The error handling at lines 81-84 will never execute—invalid or expired tokens will appear to succeed silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/auth/reset-password/controller.tsx` around lines 79 - 84, The controller's error branch is dead because model.resetPassword currently always returns true; update the logic so invalid/expired tokens are detected and resetPassword returns a meaningful boolean or throws on failure, or perform explicit token validation before calling resetPassword. Specifically, restore or implement the validation logic in the resetPassword function in app/models/user.ts (ensure it checks token existence, expiry, and matches user), or change the controller to call a new validateResetToken(token) helper and only call resetPassword when validation passes; keep the existing session.flash("error", ...) and redirect(routes.auth.resetPassword.index.href({ token: params.token })) behavior when validation fails.
🤖 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/controllers/auth/login/controller.tsx`:
- Around line 22-33: The redirect uses the user-controlled variable returnTo
directly, which allows open redirects; validate returnTo before using it by
ensuring it is an internal path (e.g., a relative path starting with "/" and not
containing a scheme or hostname) or matches your app's allowed origins, and only
use it when safe; update the login handler around the returnTo variable (the
code that sets let returnTo = url.searchParams.get("returnTo") and later calls
redirect(returnTo ?? routes.home.index.href())) to sanitize/validate returnTo
and fall back to routes.home.index.href() when validation fails.
In `@oxlint-plugins/prefer-import-alias-plugin.ts`:
- Around line 10-12: The current newSource computation only removes a single
leading "../" so nested parent imports like "../../utils/x" become
"#../utils/x"; update the logic that builds newSource (the source.replace calls
used before calling fixer.replaceText on node.source) to strip all leading "../"
occurrences (and a single "./") and then prefix the remainder with "#" — e.g.
replace the two replace(...) calls with a single expression that replaces all
leading "../" groups (use a regex like /^(\.\.\/)+/) and also handle a leading
"./" so the final newSource is "#<path-without-parent-or-dot-parts>" before
calling fixer.replaceText.
In `@server.ts`:
- Line 19: The current PORT parsing (const port = process.env.PORT ?
parseInt(process.env.PORT, 10) : 44100) can yield NaN and crash server.listen;
update the logic around process.env.PORT/parseInt to validate the parsed value
(e.g., ensure Number.isInteger(port) and port > 0 and <= 65535) and fall back to
the default 44100 (or log an error and exit) when invalid; locate and change the
code that defines the port variable (the const port line) and any server.listen
usage to use the validated port.
---
Outside diff comments:
In `@app/models/user.ts`:
- Around line 55-67: The createPasswordResetToken function calls the async
getUserByEmail(email) without awaiting it, so change createPasswordResetToken to
an async function, await getUserByEmail(email), and update its return type to
Promise<string | undefined>; inside the async function use "const user = await
getUserByEmail(email)" and keep the existing null check and token generation
logic (ensure callers of createPasswordResetToken are updated to handle a
Promise).
---
Duplicate comments:
In `@app/controllers/auth/controller.test.ts`:
- Around line 6-7: The vi.mock specifier string doesn't match the importActual
specifier causing the mock to not apply; update the vi.mock call so its first
argument string equals the string used inside importActual<typeof
import("...")>(), e.g. change vi.mock("./models/user.ts", ...) to use the exact
same module specifier used in importActual (or vice versa) so the mock pair
(vi.mock and importActual) reference the identical module name; ensure this
consistency for the vi.mock(...) and importActual<typeof import("...")>() pair
in the test file.
In `@app/controllers/auth/forgot-password/controller.tsx`:
- Around line 57-71: The current JSX renders a live password reset link when the
local variable token is present (token and routes.auth.resetPassword.index.href
are used), which leaks credentials; remove the block that outputs the anchor
with the actual href and instead replace it with a non-sensitive message (e.g.,
“If an account exists, a password reset link has been sent to your email”) or
display a masked/token-not-shown notice; ensure no code elsewhere in
forgot-password controller.tsx returns or injects the raw token into the HTML
response.
In `@app/controllers/auth/register/controller.tsx`:
- Line 118: The session is being set with the wrong shape in session.set("auth",
{ auth: user.id }) which does not match parseAuthSession (expected { userId:
string }); change the session write to use the same property name as
login/controller.tsx — call session.set("auth", { userId: user.id }) so
parseAuthSession will validate and return the newly registered user; ensure any
other places that read or write the "auth" session use the { userId } shape for
consistency with parseAuthSession.
- Line 15: The Document component on the registration page is using an incorrect
title; update the head prop in the Document usage (the line rendering Document
with url and head) to use an appropriate registration title such as "Register -
Remix Wordle" or "Sign Up - Remix Wordle" so the page title matches the
registration flow.
- Line 91: The error rendering path in the register controller uses the wrong
page title ("Login - Remix Wordle"); update the Document head title in the error
branch from "Login - Remix Wordle" to the correct "Register - Remix Wordle"
(look for the Document component instance with Document url={url}
head={<title>Login - Remix Wordle</title>}), and verify any other titles in this
file or the register error rendering path are similarly corrected to "Register -
Remix Wordle".
In `@app/controllers/auth/reset-password/controller.tsx`:
- Line 87: The success page currently uses the same Document head title ("Login
- Remix Wordle") as the login page; update the Document component used in the
reset-password success render so its head prop contains an appropriate title
like "Password Reset Success - Remix Wordle" (or "Reset Password - Remix
Wordle") instead of "Login - Remix Wordle"—look for the Document component and
its head/title usage in the reset-password controller (the line with Document
url={url} head={<title>...</title>}) and change the title string accordingly.
- Line 20: The page title in the Document component within reset-password
controller.tsx is incorrectly set to "Login"; update the head <title> used in
Document (the JSX element rendering title inside Document url={url}) to a
correct label such as "Reset Password - Remix Wordle" (or similar) so the
browser tab and accessibility metadata reflect this is the reset password page.
- Around line 79-84: The controller's error branch is dead because
model.resetPassword currently always returns true; update the logic so
invalid/expired tokens are detected and resetPassword returns a meaningful
boolean or throws on failure, or perform explicit token validation before
calling resetPassword. Specifically, restore or implement the validation logic
in the resetPassword function in app/models/user.ts (ensure it checks token
existence, expiry, and matches user), or change the controller to call a new
validateResetToken(token) helper and only call resetPassword when validation
passes; keep the existing session.flash("error", ...) and
redirect(routes.auth.resetPassword.index.href({ token: params.token })) behavior
when validation fails.
In `@app/controllers/home/controller.test.ts`:
- Around line 10-11: The vi.mock target strings do not match the typed
importActual paths, causing mocks to not apply; update each vi.mock call so its
first argument exactly matches the path used inside importActual's type-import
(change "./models/..." to "../../models/..." to mirror
importActual<...import("../../models/xxx")>), ensuring every vi.mock("...")
string equals the corresponding importActual<...import("...")> path.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: a4510ec0-677c-46e2-ac7c-d2827dfbc4cf
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (43)
.vscode/settings.jsonapp/components/document.tsxapp/components/form.tsxapp/components/game-over-modal.tsxapp/components/keyboard.tsxapp/constants.tsapp/controllers/auth/controller.test.tsapp/controllers/auth/controller.tsxapp/controllers/auth/forgot-password/controller.tsxapp/controllers/auth/login/controller.tsxapp/controllers/auth/register/controller.tsxapp/controllers/auth/reset-password/controller.tsxapp/controllers/health.tsxapp/controllers/history/controller.tsxapp/controllers/history/game.tsxapp/controllers/history/history-page.tsxapp/controllers/history/not-found-page.tsxapp/controllers/home/controller.test.tsapp/controllers/home/controller.tsxapp/controllers/home/page.tsxapp/entry.browser.tsapp/middleware/auth.tsapp/models/game.tsapp/models/user.tsapp/router.tsapp/routes.tsapp/utils/auth-session.tsapp/utils/board-to-emoji.test.tsapp/utils/db.tsapp/utils/game.test.tsapp/utils/game.tsapp/utils/queue.tsapp/utils/redirect.tsapp/utils/render.tsapp/utils/session.tsmocks/handlers.tsmocks/test.tsoxlint-plugins/prefer-import-alias-plugin.tsoxlint-plugins/prefer-let-locals-plugin.tspackage.jsonpnpm-workspace.yamlserver.tsvite.config.ts
…model functions Signed-off-by: Logan McAnsh <logan@mcan.sh>
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
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.
Summary by CodeRabbit
New Features
Documentation
Developer Experience