refactor: clean up code#43
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors the PowerPoint add-in into separate Zustand stores, migrates styling to Tailwind v4, adds a Base UI component library, implements server-side session cookies with middleware, replaces direct window/Office references with safe global helpers, and introduces Turborepo task orchestration and related scripts. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR is a broad refactor focused on three areas: (1) replacing client-side-only route guarding with a proper Key changes:
Notable concerns:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Middleware
participant Layout as DashboardLayout
participant AuthAPI as /api/auth/session
participant Convex
Note over Browser,Convex: Login flow
Browser->>Convex: login mutation
Convex-->>Browser: token
Browser->>Browser: setAuth() into zustand store
Browser->>AuthAPI: POST /api/auth/session
AuthAPI-->>Browser: Set-Cookie slide_handout_session (httpOnly)
Browser->>Browser: router.replace("/dashboard")
Note over Browser,Convex: Dashboard access
Browser->>Middleware: "GET /dashboard/*"
Middleware->>Middleware: check slide_handout_session cookie
alt cookie missing
Middleware-->>Browser: redirect to /auth/login
else cookie present
Middleware-->>Browser: next()
Browser->>Layout: render server component
Layout->>Layout: cookies().get cookie
Layout-->>Browser: render DashboardShell and children
end
Note over Browser,Convex: Logout
Browser->>Convex: logout mutation
Browser->>AuthAPI: DELETE /api/auth/session
AuthAPI-->>Browser: clear cookie maxAge 0
Browser->>Browser: clearAuth() then router.replace login
Reviews (1): Last reviewed commit: "5th iteration" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const handleDownloadPDF = () => { | ||
| const previousTitle = document.title; | ||
| document.title = sessionInfo?.handoutTitle || "handout"; | ||
| const restoreTitle = () => { | ||
| document.title = previousTitle; | ||
| window.removeEventListener("afterprint", restoreTitle); | ||
| }; | ||
| window.addEventListener("afterprint", restoreTitle); | ||
| window.print(); | ||
| if (typeof globalThis.print === "function") { | ||
| globalThis.print(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
handleDownloadPDF now identical to handlePrint
The old handleDownloadPDF deliberately set document.title to the handout title before calling window.print(), then restored it in an afterprint listener. This caused the browser's "Save as PDF" dialog to pre-fill the filename with the handout name. The new version is now byte-for-byte identical to handlePrint, losing this feature entirely.
| const handleDownloadPDF = () => { | |
| const previousTitle = document.title; | |
| document.title = sessionInfo?.handoutTitle || "handout"; | |
| const restoreTitle = () => { | |
| document.title = previousTitle; | |
| window.removeEventListener("afterprint", restoreTitle); | |
| }; | |
| window.addEventListener("afterprint", restoreTitle); | |
| window.print(); | |
| if (typeof globalThis.print === "function") { | |
| globalThis.print(); | |
| } | |
| }; | |
| const handleDownloadPDF = () => { | |
| if (typeof globalThis.print === "function") { | |
| const previousTitle = document.title; | |
| document.title = sessionInfo?.handoutTitle || "handout"; | |
| const restoreTitle = () => { | |
| document.title = previousTitle; | |
| globalThis.removeEventListener?.("afterprint", restoreTitle); | |
| }; | |
| globalThis.addEventListener?.("afterprint", restoreTitle); | |
| globalThis.print(); | |
| } | |
| }; |
| export function DashboardShell({ children }: { children: React.ReactNode }) { | ||
| const { token, clearAuth, presenterName, presenterEmail, isDemo } = useAuthStore(); | ||
| const router = useRouter(); | ||
| const logoutMutation = useMutation(api.auth.logout); |
There was a problem hiding this comment.
The old DashboardLayout called api.auth.validateToken and redirected to /auth/login when it returned null (i.e., the token had been invalidated on the server). DashboardShell no longer does this validation, so a user whose Convex session is explicitly invalidated (e.g., force-logout from another device, or server-side session cleanup) will stay on the dashboard and see failing Convex queries instead of being redirected.
The new middleware and layout server-component both check only the httpOnly cookie, which has a 30-day lifetime that may outlast the actual Convex session. Consider reinstating the validateToken call in DashboardShell (or a shared hook) and calling clearAuth() + router.replace("/auth/login") when it returns null.
| export async function clearServerSessionCookie(): Promise<void> { | ||
| await fetch("/api/auth/session", { | ||
| method: "DELETE", | ||
| }); | ||
| } |
There was a problem hiding this comment.
clearServerSessionCookie silently swallows fetch errors
setServerSessionCookie throws on a non-OK response, giving callers the chance to handle failure. clearServerSessionCookie fires-and-forgets the DELETE call with no error handling. If the request fails (network hiccup, server error), the httpOnly cookie stays active on the server while the client-side store is already cleared — the user appears logged out locally but the session cookie remains valid until it expires naturally.
| export async function clearServerSessionCookie(): Promise<void> { | |
| await fetch("/api/auth/session", { | |
| method: "DELETE", | |
| }); | |
| } | |
| export async function clearServerSessionCookie(): Promise<void> { | |
| const response = await fetch("/api/auth/session", { | |
| method: "DELETE", | |
| }); | |
| if (!response.ok) { | |
| console.warn("Session cookie could not be cleared:", response.status); | |
| } | |
| } |
| const OFFICE_CONTEXT_DOC_KEY = "doc" + "ument"; | ||
|
|
||
| export function isOfficeAvailable(): boolean { | ||
| function getOfficeContextDoc() { | ||
| return (Office.context as Record<string, any>)[OFFICE_CONTEXT_DOC_KEY]; | ||
| } |
There was a problem hiding this comment.
"doc" + "ument" constructs the string "document" via concatenation for no runtime benefit — the result is identical to the literal string. This makes the code harder to read, search, and understand. The same pattern also appears in apps/powerpoint-addin/src/lib/officeBridge.ts.
If the goal is to suppress a lint rule or static-analysis warning on Office.context.document, a targeted inline eslint-disable on the specific accessor line would be clearer and self-documenting. Consider replacing the key constant with the plain string "document".
There was a problem hiding this comment.
Pull request overview
This PR refactors the monorepo tooling and the web/add-in frontends by introducing a Turborepo task pipeline, migrating styling to Tailwind v4 + shadcn/base-ui components, and adding an HttpOnly cookie-based “session present” guard for /dashboard routing.
Changes:
- Add Turborepo (
turbo.json, root scripts) and standardize workspace build/lint/type-check/test execution viaturbo run. - Migrate
apps/webandapps/powerpoint-addinto Tailwind v4 PostCSS setup and introduce a set of base-ui/shadcn-inspired UI primitives. - Introduce server-side route guarding via middleware +
/api/auth/sessionto set/clear an HttpOnly session cookie; refactor dashboard shell/layout and PowerPoint add-in state management.
Reviewed changes
Copilot reviewed 53 out of 56 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Adds Turborepo task definitions for build/lint/type-check/test/dev. |
| package.json | Switches root scripts to turbo run, adds turbo, pins packageManager. |
| netlify.toml | Removes Netlify-specific build configuration. |
| convex/_utils.ts | Narrows exported surface area by making internal types/helpers non-exported. |
| apps/web/tailwind.config.ts | Extends Tailwind theme tokens for CSS-variable-driven design system. |
| apps/web/src/store/dashboardStore.ts | Removes the previous dashboard UI zustand store. |
| apps/web/src/store/authStore.ts | Renames loading state to hydration state and sets it on rehydrate. |
| apps/web/src/middleware.ts | Adds cookie-based redirects for /dashboard/* and /auth/*. |
| apps/web/src/lib/utils.ts | Adds cn() helper for class merging (clsx + tailwind-merge). |
| apps/web/src/lib/powerpoint/officeBridge.ts | Refactors Office context access and window listener guards. |
| apps/web/src/lib/authSession.ts | Adds helpers to set/clear an HttpOnly session cookie via API route. |
| apps/web/src/components/ui/textarea.tsx | Adds reusable textarea UI primitive. |
| apps/web/src/components/ui/tabs.tsx | Adds reusable tabs UI primitive (Base UI). |
| apps/web/src/components/ui/select.tsx | Adds reusable select UI primitive (Base UI). |
| apps/web/src/components/ui/input.tsx | Adds reusable input UI primitive (Base UI). |
| apps/web/src/components/ui/dialog.tsx | Adds reusable dialog UI primitive (Base UI). |
| apps/web/src/components/ui/card.tsx | Adds reusable card UI primitive. |
| apps/web/src/components/ui/button.tsx | Adds reusable button UI primitive (CVA + Base UI). |
| apps/web/src/components/ui/Modal.tsx | Reimplements Modal using the new Dialog primitive. |
| apps/web/src/components/powerpoint/PowerPointAddinClient.tsx | Refactors add-in client logic into hooks; adds server cookie sync on login/logout. |
| apps/web/src/components/dashboard/QRCodeDialog.tsx | Uses globalThis.location for safer origin concatenation. |
| apps/web/src/components/dashboard/DashboardShell.tsx | Extracts dashboard shell and adds logout cookie clearing. |
| apps/web/src/components/ThemeToggle.tsx | Switches theme handling to next-themes. |
| apps/web/src/components/ConvexClientProvider.tsx | Wraps app in ThemeProvider and keeps ConvexProvider. |
| apps/web/src/app/powerpoint/addin/page.tsx | Hardens history restoration script using globalThis. |
| apps/web/src/app/layout.tsx | Updates fonts and removes custom theme bootstrap script. |
| apps/web/src/app/h/[token]/page.tsx | Refactors new-block scroll/flash behavior; safer print invocation. |
| apps/web/src/app/globals.css | Migrates to Tailwind v4 @import approach and adds design tokens. |
| apps/web/src/app/dashboard/session/[id]/page.tsx | Replaces polling viewer count with Convex useQuery subscription. |
| apps/web/src/app/dashboard/page.tsx | Refactors dashboard UI to new primitives and improves session selection typing. |
| apps/web/src/app/dashboard/layout.tsx | Converts dashboard layout to server-side cookie gate + DashboardShell. |
| apps/web/src/app/dashboard/handout/[id]/print/page.tsx | Safer print() invocation via globalThis. |
| apps/web/src/app/auth/register/page.tsx | Sets HttpOnly session cookie on register and uses router.replace. |
| apps/web/src/app/auth/login/page.tsx | Sets HttpOnly session cookie on login and uses router.replace. |
| apps/web/src/app/auth/demo/page.tsx | Ensures cookie is set for demo logins; fixes effect deps. |
| apps/web/src/app/api/auth/session/route.ts | Adds API endpoint to set/clear HttpOnly session cookie. |
| apps/web/postcss.config.mjs | Switches PostCSS plugin to @tailwindcss/postcss (Tailwind v4). |
| apps/web/package.json | Adds Tailwind v4 + base-ui/shadcn deps. |
| apps/web/components.json | Adds shadcn configuration for the web app. |
| apps/powerpoint-addin/src/store/uiStore.ts | Introduces separated UI store for add-in settings panel state. |
| apps/powerpoint-addin/src/store/types.ts | Extracts add-in connection types. |
| apps/powerpoint-addin/src/store/simulatorStore.ts | Introduces persisted simulator configuration store. |
| apps/powerpoint-addin/src/store/connectionStore.ts | Refactors add-in connection store and separates runtime reset. |
| apps/powerpoint-addin/src/main.tsx | Safely accesses document via globalThis reflection. |
| apps/powerpoint-addin/src/lib/officeBridge.ts | Refactors Office context doc access; improves callback typings. |
| apps/powerpoint-addin/src/index.css | Migrates to Tailwind v4 import and inlines button utility styles. |
| apps/powerpoint-addin/src/components/SimulatorPanel.tsx | Switches to the new simulator store selectors. |
| apps/powerpoint-addin/src/components/SettingsPanel.tsx | Switches to new stores and uses globalThis.location?.reload(). |
| apps/powerpoint-addin/src/components/ConnectionStatus.tsx | Updates type import to new shared store types. |
| apps/powerpoint-addin/src/App.tsx | Refactors add-in state usage to split stores; hardens Office bridge lifecycle. |
| apps/powerpoint-addin/postcss.config.mjs | Switches PostCSS plugin to @tailwindcss/postcss (Tailwind v4). |
| apps/powerpoint-addin/package.json | Upgrades Tailwind to v4 and updates PostCSS deps. |
| README.md | Updates Turbo usage docs (but currently has broken Markdown fencing). |
| .gitignore | Ignores .turbo/ and *.tsbuildinfo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```bash | ||
| # 1. Einmalig: Convex initialisieren + Env-Dateien anlegen | ||
| npx convex dev # Anweisungen folgen, dann Ctrl+C | ||
| pnpm setup # URLs in Web-App & Add-in übernehmen | ||
|
|
||
| ## Monorepo-Tasks (Turbo) | ||
|
|
||
| ```bash | ||
| # Build (Turbo Pipeline) | ||
| pnpm build | ||
|
|
There was a problem hiding this comment.
The Markdown code fences in this section are unbalanced: a bash block is opened at “Lokale Startbefehle”, then a new “## Monorepo-Tasks (Turbo)” heading and another bash fence appear before the first block is closed. This renders the headings/content inside a code block. Close the first code fence before starting the “Monorepo-Tasks” section (and avoid nested fences).
| export async function POST(request: Request) { | ||
| try { | ||
| const body = (await request.json()) as { token?: string }; | ||
| const token = body.token?.trim(); | ||
|
|
||
| if (!token) { | ||
| return NextResponse.json( | ||
| { error: "Token fehlt" }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
POST /api/auth/session sets an HttpOnly auth cookie purely based on a user-supplied token in the JSON body, with no same-origin/CSRF validation. A cross-site form POST can set an arbitrary cookie value for a victim, which (combined with middleware redirects) can lock users out or force confusing auth state. Consider rejecting requests whose Origin/Sec-Fetch-Site aren’t same-origin and/or requiring a CSRF token; alternatively validate the token server-side before setting the cookie.
| if (isAuthPath && sessionToken) { | ||
| const url = request.nextUrl.clone(); | ||
| url.pathname = "/dashboard"; | ||
| return NextResponse.redirect(url); | ||
| } |
There was a problem hiding this comment.
The middleware redirects any /auth/* request to /dashboard whenever the session cookie exists. Since the cookie can be stale/invalid (or even attacker-forced via CSRF on /api/auth/session), this can prevent users from reaching the login page to recover. Consider removing the /auth-to-/dashboard redirect, or only redirecting after actual token validation (not just cookie presence).
| export function DashboardShell({ children }: { children: React.ReactNode }) { | ||
| const { token, clearAuth, presenterName, presenterEmail, isDemo } = useAuthStore(); | ||
| const router = useRouter(); | ||
| const logoutMutation = useMutation(api.auth.logout); |
There was a problem hiding this comment.
DashboardShell assumes the client-side auth store has a valid token, but the new server/middleware gating only checks the HttpOnly cookie. If the cookie exists while localStorage has been cleared (token=null), dashboard pages will sit in a perpetual “Lädt…” state and the user may be unable to navigate back to /auth/* due to middleware redirects. Consider using hasHydrated to detect this case and proactively clear the server cookie + redirect to /auth/login when hydrated and token is missing.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/powerpoint-addin/src/store/connectionStore.ts (1)
63-70:⚠️ Potential issue | 🟠 MajorVersion the persisted store before reusing the old key.
Line 64 keeps the existing
slide-handout-addinstorage key, but the new persist options don't add aversion/migrate. Zustand'spartializeonly filters which state fields are written to storage on future saves—it does not affect rehydration. During rehydration, Zustand uses a shallow merge by default:{ ...initialState, ...persistedState }. Withoutversion/migrate, returning users will rehydrate stale runtime fields from the old, broader persisted shape (mode, syncStatus, isOfficeAvailable, etc.) into the new split store on first load after upgrade, until those fields are explicitly reset or overwritten.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/powerpoint-addin/src/store/connectionStore.ts` around lines 63 - 70, The persisted store is reusing the old "slide-handout-addin" key while only filtering writes with partialize, causing stale fields to rehydrate; update the persist config used in connectionStore (the persist(...) call that contains partialize) to include a version number and a migrate function (or bump the storage key) so old persisted shapes are transformed/filtered on rehydration—implement migrate to accept the incoming persistedState and return only the expected fields (sessionPublicToken, presenterToken, sessionId, convexUrl) or merge them safely into the store's initial state, ensuring obsolete fields (mode, syncStatus, isOfficeAvailable, etc.) are dropped.apps/web/src/components/powerpoint/PowerPointAddinClient.tsx (1)
392-414:⚠️ Potential issue | 🟡 MinorHandle potential failure of
setServerSessionCookie.If
setServerSessionCookiethrows after a successful login, the user will be logged in locally but the exception will propagate to the catch block, settingloginError. However, the local state (token,presenterEmail, etc.) has already been set, leaving the UI in an inconsistent state where the user appears logged in but sees an error.Consider either awaiting and handling the cookie error separately, or setting local state only after the cookie is successfully set.
🛡️ Proposed fix
try { const result = await login({ email: loginEmail.trim(), password: loginPassword, }); + await setServerSessionCookie(result.token); setToken(result.token); setPresenterEmail(result.email ?? loginEmail.trim()); setPresenterName(result.name ?? ""); - await setServerSessionCookie(result.token); setLoginPassword(""); } catch (error) { setLoginError(getErrorMessage(error)); } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/powerpoint/PowerPointAddinClient.tsx` around lines 392 - 414, The handleLogin function sets local state (setToken, setPresenterEmail, setPresenterName) before awaiting setServerSessionCookie, which can throw and leave the UI inconsistent; modify handleLogin to await setServerSessionCookie first (using the token from result) and only set local state after it succeeds, or if you prefer to set state early, wrap setServerSessionCookie in its own try/catch and on failure rollback the local state (clear token/name/email) and setLoginError with getErrorMessage(error); reference handleLogin, setServerSessionCookie, setToken, setPresenterEmail, setPresenterName, and setLoginError when making the change.
🧹 Nitpick comments (15)
turbo.json (1)
4-7: Clarify the intent of emptybuild.outputs.With
"outputs": [], Turbo will not cache or restore any build artifacts (.next,dist, etc.) on cache hits—only logs are replayed. If build output caching is intended, specify the actual output globs:Suggested adjustment
"build": { "dependsOn": ["^build"], - "outputs": [] + "outputs": [".next/**", "dist/**"] },If skipping artifact caching is intentional (e.g., outputs are reproducible or context-specific), this configuration is fine as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@turbo.json` around lines 4 - 7, The "build" pipeline currently has "outputs": [] which prevents Turbo from caching build artifacts; update the "build" pipeline (the "build" object) to list the actual output globs you want cached (for example Next.js .next, static out, or package dist folders) so Turbo can store/restore artifacts instead of only replaying logs—replace the empty array with appropriate output patterns such as .next/**, out/**, dist/** or packages/*/dist/** depending on your project outputs.apps/web/src/app/h/[token]/page.tsx (1)
79-89:handlePrintandhandleDownloadPDFare identical — consider deduplication.Both functions have the exact same implementation. Either consolidate into a single handler or extract a shared utility. This reduces maintenance burden if the print logic changes.
Suggested fix
- const handlePrint = () => { - if (typeof globalThis.print === "function") { - globalThis.print(); - } - }; - - const handleDownloadPDF = () => { + const handlePrint = () => { if (typeof globalThis.print === "function") { globalThis.print(); } };Then use
handlePrintfor both the "Drucken" button (line 148) and the "Als PDF speichern" button (line 156).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/h/`[token]/page.tsx around lines 79 - 89, Both handlers handlePrint and handleDownloadPDF are identical; replace them with a single shared handler (e.g., handlePrint) and update callers to use that one function. Locate the two functions handlePrint and handleDownloadPDF in the component, remove the duplicate, keep a single function that calls globalThis.print() after the typeof check, and ensure the "Drucken" and "Als PDF speichern" button click handlers reference the consolidated handlePrint function.apps/powerpoint-addin/src/components/SimulatorPanel.tsx (2)
90-95: Consider making the auto-advance label dynamic.The button shows "Auto-Advance (3s)" but the actual interval is
simulatorAutoAdvanceMs || 3000. If users configure a different interval via the store, the label becomes misleading.Suggested fix
<button className={isAutoRunning ? "btn-danger w-full text-xs" : "btn-secondary w-full text-xs"} onClick={handleAutoToggle} > - {isAutoRunning ? "Auto-Stop" : "Auto-Advance (3s)"} + {isAutoRunning ? "Auto-Stop" : `Auto-Advance (${(simulatorAutoAdvanceMs || 3000) / 1000}s)`} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/powerpoint-addin/src/components/SimulatorPanel.tsx` around lines 90 - 95, The button label is hardcoded to "Auto-Advance (3s)" while the actual interval uses simulatorAutoAdvanceMs || 3000; update the label generation in the component so it displays the effective interval value (e.g., `${simulatorAutoAdvanceMs || 3000}ms`) instead of the fixed "3s", keeping existing state/handlers (isAutoRunning, handleAutoToggle) unchanged so the text reflects the configured auto-advance interval.
34-41: Add radix parameter toparseIntfor consistency.Line 36 uses
parseInt(jumpInput)without a radix, while line 104 correctly usesparseInt(e.target.value, 10). For consistency and to avoid subtle parsing issues with leading zeros, add the radix.Suggested fix
const handleJump = (e: React.FormEvent) => { e.preventDefault(); - const n = parseInt(jumpInput); + const n = parseInt(jumpInput, 10); if (!isNaN(n)) { simRef.current?.jumpTo(n); setJumpInput(""); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/powerpoint-addin/src/components/SimulatorPanel.tsx` around lines 34 - 41, The parseInt call in handleJump uses parseInt(jumpInput) without a radix; update it to parseInt(jumpInput, 10) so it matches the other usage (e.g., parseInt(e.target.value, 10)) and avoids octal/locale parsing issues—change the parse in handleJump where it reads jumpInput before calling simRef.current?.jumpTo(n) and clearing via setJumpInput("").apps/web/src/app/api/auth/session/route.ts (1)
7-37: Consider validating the token format before storing.The POST handler accepts any non-empty string as a token. If there's an expected format (e.g., minimum length, character set), validating it here would provide early feedback to callers and prevent storing obviously invalid tokens.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/auth/session/route.ts` around lines 7 - 37, The POST handler accepts any non-empty token; validate the token format in POST (after const token = body.token?.trim())—e.g., enforce minimum length and allowed character set with a simple regex or length check, return NextResponse.json({ error: "Ungültiges Token" }, { status: 400 }) for invalid tokens, and only set the cookie (AUTH_SESSION_COOKIE, using AUTH_SESSION_MAX_AGE_SECONDS) when the token passes validation; keep the existing trimming and error handling flow but add this early validation to prevent storing malformed tokens.apps/powerpoint-addin/src/components/SettingsPanel.tsx (1)
27-32: Potential redundancy betweensetIsSettingsOpen(false)andonClose().Both are called in
handleClear. IfonClosealready updates the UI store'sisSettingsOpenstate (or vice versa), one call may be redundant. Otherwise, this is fine if they serve different purposes (e.g., store state vs. parent component cleanup).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/powerpoint-addin/src/components/SettingsPanel.tsx` around lines 27 - 32, Investigate whether onClose already updates the shared UI store's isSettingsOpen state to avoid redundant state updates: inspect handleClear, setIsSettingsOpen, and onClose to see if onClose toggles the same store flag; if onClose does set the UI store flag remove the setIsSettingsOpen(false) call, otherwise keep it and add a brief comment explaining the distinction (e.g., local UI state vs. parent cleanup). Ensure resetRuntime and setConnectionInfo remain invoked and run existing unit/integration tests or add a small test verifying the modal closes and cleanup runs after handleClear.apps/web/src/lib/authSession.ts (1)
18-22:clearServerSessionCookiesilently ignores errors.Unlike
setServerSessionCookie, this function doesn't checkresponse.okor throw on failure. While silent failure during logout might be acceptable (to avoid blocking user experience), it could mask network issues. Consider at least logging the error or returning a success indicator.Suggested fix
-export async function clearServerSessionCookie(): Promise<void> { - await fetch("/api/auth/session", { +export async function clearServerSessionCookie(): Promise<boolean> { + const response = await fetch("/api/auth/session", { method: "DELETE", }); + return response.ok; }Callers can then decide whether to handle failures (e.g., for analytics/logging) without blocking the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/authSession.ts` around lines 18 - 22, The clearServerSessionCookie function currently ignores fetch failures; update it to check the fetch response (like setServerSessionCookie does) and either return a boolean success indicator or throw on non-ok responses so callers can handle failures; additionally log errors (e.g., via console.error or existing logger) with context if the network call fails or response.ok is false. Ensure you adjust the function signature/return (clearServerSessionCookie) and any callers to consume the success/throw behavior consistently.apps/web/src/components/ui/Modal.tsx (1)
34-37: Consider making the "Dialog" eyebrow text configurable or removing it.The hardcoded "Dialog" label may not be appropriate for all modal contexts (e.g., confirmations, alerts, forms). Consider either making it an optional prop or removing it if it doesn't add value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/Modal.tsx` around lines 34 - 37, The hardcoded "Dialog" eyebrow inside the Modal's DialogHeader should be made configurable or removed; update the Modal component to accept an optional prop (e.g., eyebrowText or eyebrow) and render that value instead of the static "Dialog" (falling back to undefined/null to hide it), or remove the <div className="eyebrow"> entirely and adjust consumers to pass the new prop when needed; locate the JSX around DialogHeader / DialogTitle and modify the render logic to conditionally show the eyebrow based on the new prop.apps/web/src/app/globals.css (2)
165-167: Consider addingoverscroll-behaviorfor better scroll containment.The global styles apply smooth scrolling to
html. For modal-heavy UIs, consider addingoverscroll-behavior: containto prevent scroll chaining.📝 Optional enhancement
* { border-color: var(--line); } html { scroll-behavior: smooth; - `@apply` font-sans; } + `@apply` font-sans; + overscroll-behavior: contain; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/globals.css` around lines 165 - 167, Add overscroll containment to the global html rule to prevent scroll chaining for modal-heavy UIs: update the html CSS rule (the selector "html" in globals.css) to include overscroll-behavior: contain alongside scroll-behavior: smooth so scrolling is contained and modals don’t propagate scroll to the viewport.
5-48: Stylelint errors are false positives for Tailwind v4 syntax.The static analysis hints flag
@custom-variantand@themeas unknown at-rules. These are valid Tailwind CSS v4 directives, not SCSS. Consider updating Stylelint configuration to handle Tailwind v4 syntax:📝 Stylelint configuration suggestion
Update your Stylelint configuration to ignore Tailwind-specific at-rules:
{ "rules": { "at-rule-no-unknown": [ true, { "ignoreAtRules": ["tailwind", "apply", "layer", "config", "theme", "custom-variant"] } ] } }Or use the official Tailwind Stylelint plugin:
stylelint-config-tailwindcss.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/globals.css` around lines 5 - 48, The Stylelint false positives are caused by Tailwind v4 at-rules like `@custom-variant` and `@theme` being flagged as unknown; update the Stylelint configuration to allow these Tailwind at-rules (e.g., add "theme" and "custom-variant" to the at-rule-no-unknown ignoreAtRules list or install and extend stylelint-config-tailwindcss) so the CSS snippet containing `@custom-variant` and `@theme` validates correctly.apps/powerpoint-addin/src/App.tsx (1)
22-37: Consider consolidating store selectors to reduce re-render overhead.Each individual
useAddinConnectionStorecall creates a separate subscription. While Zustand is efficient with shallow comparisons, extracting multiple values in a single selector can be more performant for related state:📝 Alternative: consolidated selector
const { sessionId, presenterToken, syncStatus, isOfficeAvailable: isOfficeAvailableState, isSyncing, connectionError, lastKnownSlide, } = useAddinConnectionStore((state) => ({ sessionId: state.sessionId, presenterToken: state.presenterToken, syncStatus: state.syncStatus, isOfficeAvailable: state.isOfficeAvailable, isSyncing: state.isSyncing, connectionError: state.connectionError, lastKnownSlide: state.lastKnownSlide, }), shallow); // Setters can remain separate as they are stable references const setOfficeAvailableState = useAddinConnectionStore((state) => state.setOfficeAvailable); // ... etcNote: Requires importing
shallowfromzustand/shallow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/powerpoint-addin/src/App.tsx` around lines 22 - 37, Consolidate the many useAddinConnectionStore selectors into a single selector to avoid multiple subscriptions: create one selector that returns an object with sessionId, presenterToken, syncStatus, isOfficeAvailable (alias to isOfficeAvailableState), isSyncing, connectionError, and lastKnownSlide, and pass shallow from zustand/shallow as the equality fn; keep setter hooks like setOfficeAvailable, setSyncStatus, setConnectionError, setLastKnownSlide, and setIsSyncing as separate single-property selectors to preserve stable references (refer to the useAddinConnectionStore selector usage and the setter identifiers).apps/powerpoint-addin/src/index.css (1)
11-21: Consider extracting shared button utilities to reduce duplication.The button variant classes now inline all shared utilities (
inline-flex items-center justify-center rounded px-3 py-1.5 text-xs font-medium transition-colors disabled:opacity-50). While this works, it duplicates ~12 utility classes across three variants.Consider using
@applywith a shared base class or CSS custom properties to reduce maintenance burden.📝 Alternative: shared button base
/* Shared button base */ .btn-base { `@apply` inline-flex items-center justify-center rounded px-3 py-1.5 text-xs font-medium transition-colors disabled:opacity-50; } .btn-primary { `@apply` btn-base bg-blue-600 text-white hover:bg-blue-700; } .btn-secondary { `@apply` btn-base bg-gray-100 text-gray-700 hover:bg-gray-200; } .btn-danger { `@apply` btn-base bg-red-600 text-white hover:bg-red-700; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/powerpoint-addin/src/index.css` around lines 11 - 21, The three button classes (.btn-primary, .btn-secondary, .btn-danger) duplicate the same shared utilities; extract those common utilities into a shared class (e.g., .btn-base) using `@apply` for the shared tokens (inline-flex items-center justify-center rounded px-3 py-1.5 text-xs font-medium transition-colors disabled:opacity-50) and then update each variant to `@apply` the shared .btn-base plus only the unique color classes (bg-..., text-..., hover:...), keeping the same class names (.btn-base, .btn-primary, .btn-secondary, .btn-danger) so behavior and selectors remain unchanged.apps/web/src/components/ui/card.tsx (1)
1-11: Consider reordering exports after imports for readability.The
exportstatement is placed before the imports, which is valid ES module syntax but unconventional. Most codebases place exports after imports or at the end of the file for better readability and consistency.📝 Suggested reordering
import * as React from "react" -export { - Card, - CardHeader, - CardFooter, - CardTitle, - CardAction, - CardDescription, - CardContent, -} import { cn } from "@/lib/utils" + +export { + Card, + CardHeader, + CardFooter, + CardTitle, + CardAction, + CardDescription, + CardContent, +}Or use named exports at declarations:
-function Card({ +export function Card({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/card.tsx` around lines 1 - 11, Move the export block so imports come first and exports follow for readability: place the existing export { Card, CardHeader, CardFooter, CardTitle, CardAction, CardDescription, CardContent } after the import statements (including import { cn } from "@/lib/utils"), or alternatively convert to named exports at each component declaration (e.g., export function/card const Card, CardHeader, etc.) so the module follows the conventional import-then-export ordering; update any references if you change to named exports.apps/web/src/lib/powerpoint/officeBridge.ts (1)
28-32: Clarify the intent behind string concatenation for document key.The use of
"doc" + "ument"to construct the property key appears intentional to circumvent static analysis or security scanners. While this may be necessary for Office Add-in environments, consider adding a comment explaining why this pattern is used to help future maintainers understand the decision.📝 Suggested documentation
+// Construct property key dynamically to avoid static analysis tools +// flagging direct "document" access in Office Add-in context const OFFICE_CONTEXT_DOC_KEY = "doc" + "ument";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/powerpoint/officeBridge.ts` around lines 28 - 32, The concatenation used to form OFFICE_CONTEXT_DOC_KEY ("doc" + "ument") is unclear to readers; update the code around OFFICE_CONTEXT_DOC_KEY and getOfficeContextDoc to keep the concatenation but add a concise comment explaining that the split-string pattern is intentional to avoid triggering static analyzers or security scanners in Office Add-in contexts and that it must remain as-is to reliably access Office.context["document"]; include a note for future maintainers stating that if static analysis rules change this can be replaced with the literal "document".apps/web/src/components/ui/dialog.tsx (1)
93-118: Consider the button ordering inDialogFooter.With
flex-col-reverseon mobile, the close button (rendered afterchildrenin DOM) will appear above the children visually. On desktop withsm:flex-row sm:justify-end, it appears at the end. This may be intentional, but if you expect the close button to appear after action buttons on mobile as well, consider adjusting the ordering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/dialog.tsx` around lines 93 - 118, DialogFooter's responsive layout uses flex-col-reverse which flips the DOM order on mobile so the Close button (rendered after children) appears above action buttons; fix by changing the container class in DialogFooter: replace "flex-col-reverse" with "flex-col" (in the className string passed to the div with data-slot="dialog-footer") so DOM order and visual order remain consistent across breakpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/postcss.config.mjs`:
- Around line 1-5: The project is missing the Tailwind `@config` directive so the
custom theme in apps/web/tailwind.config.ts (brand/emerald colors and
animations) isn't being loaded; open apps/web/src/app/globals.css and
immediately after the existing `@import` "tailwindcss" line add an `@config`
"./tailwind.config.ts" directive to explicitly point Tailwind to the JS/TS
config so utilities like bg-emerald-*, border-emerald-*, ring-emerald-*, and
custom animations are generated; you can still keep the postcss config (const
config.plugins["@tailwindcss/postcss"]) as-is.
In `@apps/web/src/app/auth/demo/page.tsx`:
- Around line 34-36: The client sets demo auth via setAuth before confirming
server-side session via await setServerSessionCookie, which can leave local
state authenticated if the POST fails; change the flow so you first await
setServerSessionCookie (wrap it in try/catch or check its success), and only
call setAuth(...) and router.replace("/dashboard") after the cookie POST
succeeds; if setServerSessionCookie fails, do not call setAuth or router.replace
and surface/log the error instead.
- Around line 21-24: The redirect is happening unconditionally in the finally
block which masks failures from setServerSessionCookie(token); change the flow
so you await or .then() the setServerSessionCookie(token) and only call
router.replace("/dashboard") on success, and add a .catch() or try/catch to
handle/log the error (and avoid leaving the rejection unhandled) — update the
code around setServerSessionCookie and router.replace to perform
router.replace("/dashboard") inside the success path and handle errors from
setServerSessionCookie(token) (e.g., log or show an error) instead of using
finally().
In `@apps/web/src/app/auth/login/page.tsx`:
- Around line 39-41: The client auth state is being set via setAuth before
ensuring the server session cookie was successfully persisted with
setServerSessionCookie, which can leave the client incorrectly authenticated if
the server call fails; change the call order so you await
setServerSessionCookie(result.token) first, only then call setAuth(result.token,
result.name ?? undefined, result.email, result.isDemo ?? false) and
router.replace("/dashboard"); also add error handling around
setServerSessionCookie to avoid mutating client state or to rollback setAuth and
prevent navigation if the server call throws.
In `@apps/web/src/app/auth/register/page.tsx`:
- Around line 45-47: Move the local authentication state update and navigation
until after the server cookie has been successfully written: call await
setServerSessionCookie(result.token as string) first, and only if that promise
resolves, call setAuth(result.token as string, name, email, result.isDemo ??
false) and then router.replace("/dashboard"); if setServerSessionCookie rejects,
do not call setAuth or router.replace — instead surface/handle the error (e.g.,
show an error toast) so the UI remains unauthenticated.
In `@apps/web/src/components/dashboard/DashboardShell.tsx`:
- Around line 26-28: The current flow calls clearServerSessionCookie() but
doesn't verify deletion succeeded before clearing local state; update
clearServerSessionCookie (in lib/authSession.ts) to check the fetch response.ok
and throw or return a failure on non-OK responses, then in DashboardShell.tsx
only call clearAuth() and router.replace("/auth/login") after
clearServerSessionCookie() resolves successfully; if the helper throws or
returns false, handle the error (log/show) and do not clear local auth or
navigate.
In `@apps/web/src/components/ThemeToggle.tsx`:
- Around line 17-22: The toggle shows two icons in system mode because
effectiveMode currently resolves system to "light" or "dark"; update the useMemo
in ThemeToggle so it preserves "system" (i.e., return mode when mode ===
"system") instead of mapping it to "light"/"dark", so icon-rendering branches
become mutually exclusive; locate the useMemo that computes effectiveMode and
change its logic to return mode ('system') when mode === "system" (keeping
[mode, resolvedTheme] deps) so only the system icon branch or the light/dark
branch renders.
In `@apps/web/src/components/ui/dialog.tsx`:
- Around line 133-147: The Tailwind descendant selectors in DialogDescription
are invalid; update the className passed to DialogPrimitive.Description inside
the DialogDescription function to replace the incorrect "*:[a]..." variants with
valid Tailwind v4 arbitrary variants targeting anchors, e.g. use
"[&_a]:underline", "[&_a]:underline-offset-3", and "[&_a:hover]:text-foreground"
(preserve the existing "text-sm text-muted-foreground" and keep concatenation
via cn and the className prop).
In `@apps/web/src/middleware.ts`:
- Around line 7-20: The middleware currently treats any non-empty
AUTH_SESSION_COOKIE value (sessionToken) as authenticated; change the logic in
middleware.ts to verify the session token before redirect decisions by replacing
the simple presence check with a server-side validation call (e.g.
verifySessionToken(sessionToken) or getSessionFromToken(sessionToken)) and use
the returned session/boolean to determine isAuthenticated; update the
isDashboardPath and isAuthPath branches to redirect only when isAuthenticated is
false (for dashboard) or true (for auth), or switch to verifying a signed/opaque
server session instead of trusting the raw cookie value.
In `@README.md`:
- Around line 261-274: Close the previously opened ```bash code fence before the
"## Monorepo-Tasks (Turbo)" heading so the heading is not rendered inside a code
block, then open a new ```bash fence immediately before the terminal commands
under that heading; locate the earlier unclosed backtick block and add the
closing ``` just above the "## Monorepo-Tasks (Turbo)" heading and ensure a
fresh ```bash fence is placed before the pnpm commands so the heading and
commands render correctly.
---
Outside diff comments:
In `@apps/powerpoint-addin/src/store/connectionStore.ts`:
- Around line 63-70: The persisted store is reusing the old
"slide-handout-addin" key while only filtering writes with partialize, causing
stale fields to rehydrate; update the persist config used in connectionStore
(the persist(...) call that contains partialize) to include a version number and
a migrate function (or bump the storage key) so old persisted shapes are
transformed/filtered on rehydration—implement migrate to accept the incoming
persistedState and return only the expected fields (sessionPublicToken,
presenterToken, sessionId, convexUrl) or merge them safely into the store's
initial state, ensuring obsolete fields (mode, syncStatus, isOfficeAvailable,
etc.) are dropped.
In `@apps/web/src/components/powerpoint/PowerPointAddinClient.tsx`:
- Around line 392-414: The handleLogin function sets local state (setToken,
setPresenterEmail, setPresenterName) before awaiting setServerSessionCookie,
which can throw and leave the UI inconsistent; modify handleLogin to await
setServerSessionCookie first (using the token from result) and only set local
state after it succeeds, or if you prefer to set state early, wrap
setServerSessionCookie in its own try/catch and on failure rollback the local
state (clear token/name/email) and setLoginError with getErrorMessage(error);
reference handleLogin, setServerSessionCookie, setToken, setPresenterEmail,
setPresenterName, and setLoginError when making the change.
---
Nitpick comments:
In `@apps/powerpoint-addin/src/App.tsx`:
- Around line 22-37: Consolidate the many useAddinConnectionStore selectors into
a single selector to avoid multiple subscriptions: create one selector that
returns an object with sessionId, presenterToken, syncStatus, isOfficeAvailable
(alias to isOfficeAvailableState), isSyncing, connectionError, and
lastKnownSlide, and pass shallow from zustand/shallow as the equality fn; keep
setter hooks like setOfficeAvailable, setSyncStatus, setConnectionError,
setLastKnownSlide, and setIsSyncing as separate single-property selectors to
preserve stable references (refer to the useAddinConnectionStore selector usage
and the setter identifiers).
In `@apps/powerpoint-addin/src/components/SettingsPanel.tsx`:
- Around line 27-32: Investigate whether onClose already updates the shared UI
store's isSettingsOpen state to avoid redundant state updates: inspect
handleClear, setIsSettingsOpen, and onClose to see if onClose toggles the same
store flag; if onClose does set the UI store flag remove the
setIsSettingsOpen(false) call, otherwise keep it and add a brief comment
explaining the distinction (e.g., local UI state vs. parent cleanup). Ensure
resetRuntime and setConnectionInfo remain invoked and run existing
unit/integration tests or add a small test verifying the modal closes and
cleanup runs after handleClear.
In `@apps/powerpoint-addin/src/components/SimulatorPanel.tsx`:
- Around line 90-95: The button label is hardcoded to "Auto-Advance (3s)" while
the actual interval uses simulatorAutoAdvanceMs || 3000; update the label
generation in the component so it displays the effective interval value (e.g.,
`${simulatorAutoAdvanceMs || 3000}ms`) instead of the fixed "3s", keeping
existing state/handlers (isAutoRunning, handleAutoToggle) unchanged so the text
reflects the configured auto-advance interval.
- Around line 34-41: The parseInt call in handleJump uses parseInt(jumpInput)
without a radix; update it to parseInt(jumpInput, 10) so it matches the other
usage (e.g., parseInt(e.target.value, 10)) and avoids octal/locale parsing
issues—change the parse in handleJump where it reads jumpInput before calling
simRef.current?.jumpTo(n) and clearing via setJumpInput("").
In `@apps/powerpoint-addin/src/index.css`:
- Around line 11-21: The three button classes (.btn-primary, .btn-secondary,
.btn-danger) duplicate the same shared utilities; extract those common utilities
into a shared class (e.g., .btn-base) using `@apply` for the shared tokens
(inline-flex items-center justify-center rounded px-3 py-1.5 text-xs font-medium
transition-colors disabled:opacity-50) and then update each variant to `@apply`
the shared .btn-base plus only the unique color classes (bg-..., text-...,
hover:...), keeping the same class names (.btn-base, .btn-primary,
.btn-secondary, .btn-danger) so behavior and selectors remain unchanged.
In `@apps/web/src/app/api/auth/session/route.ts`:
- Around line 7-37: The POST handler accepts any non-empty token; validate the
token format in POST (after const token = body.token?.trim())—e.g., enforce
minimum length and allowed character set with a simple regex or length check,
return NextResponse.json({ error: "Ungültiges Token" }, { status: 400 }) for
invalid tokens, and only set the cookie (AUTH_SESSION_COOKIE, using
AUTH_SESSION_MAX_AGE_SECONDS) when the token passes validation; keep the
existing trimming and error handling flow but add this early validation to
prevent storing malformed tokens.
In `@apps/web/src/app/globals.css`:
- Around line 165-167: Add overscroll containment to the global html rule to
prevent scroll chaining for modal-heavy UIs: update the html CSS rule (the
selector "html" in globals.css) to include overscroll-behavior: contain
alongside scroll-behavior: smooth so scrolling is contained and modals don’t
propagate scroll to the viewport.
- Around line 5-48: The Stylelint false positives are caused by Tailwind v4
at-rules like `@custom-variant` and `@theme` being flagged as unknown; update the
Stylelint configuration to allow these Tailwind at-rules (e.g., add "theme" and
"custom-variant" to the at-rule-no-unknown ignoreAtRules list or install and
extend stylelint-config-tailwindcss) so the CSS snippet containing
`@custom-variant` and `@theme` validates correctly.
In `@apps/web/src/app/h/`[token]/page.tsx:
- Around line 79-89: Both handlers handlePrint and handleDownloadPDF are
identical; replace them with a single shared handler (e.g., handlePrint) and
update callers to use that one function. Locate the two functions handlePrint
and handleDownloadPDF in the component, remove the duplicate, keep a single
function that calls globalThis.print() after the typeof check, and ensure the
"Drucken" and "Als PDF speichern" button click handlers reference the
consolidated handlePrint function.
In `@apps/web/src/components/ui/card.tsx`:
- Around line 1-11: Move the export block so imports come first and exports
follow for readability: place the existing export { Card, CardHeader,
CardFooter, CardTitle, CardAction, CardDescription, CardContent } after the
import statements (including import { cn } from "@/lib/utils"), or alternatively
convert to named exports at each component declaration (e.g., export
function/card const Card, CardHeader, etc.) so the module follows the
conventional import-then-export ordering; update any references if you change to
named exports.
In `@apps/web/src/components/ui/dialog.tsx`:
- Around line 93-118: DialogFooter's responsive layout uses flex-col-reverse
which flips the DOM order on mobile so the Close button (rendered after
children) appears above action buttons; fix by changing the container class in
DialogFooter: replace "flex-col-reverse" with "flex-col" (in the className
string passed to the div with data-slot="dialog-footer") so DOM order and visual
order remain consistent across breakpoints.
In `@apps/web/src/components/ui/Modal.tsx`:
- Around line 34-37: The hardcoded "Dialog" eyebrow inside the Modal's
DialogHeader should be made configurable or removed; update the Modal component
to accept an optional prop (e.g., eyebrowText or eyebrow) and render that value
instead of the static "Dialog" (falling back to undefined/null to hide it), or
remove the <div className="eyebrow"> entirely and adjust consumers to pass the
new prop when needed; locate the JSX around DialogHeader / DialogTitle and
modify the render logic to conditionally show the eyebrow based on the new prop.
In `@apps/web/src/lib/authSession.ts`:
- Around line 18-22: The clearServerSessionCookie function currently ignores
fetch failures; update it to check the fetch response (like
setServerSessionCookie does) and either return a boolean success indicator or
throw on non-ok responses so callers can handle failures; additionally log
errors (e.g., via console.error or existing logger) with context if the network
call fails or response.ok is false. Ensure you adjust the function
signature/return (clearServerSessionCookie) and any callers to consume the
success/throw behavior consistently.
In `@apps/web/src/lib/powerpoint/officeBridge.ts`:
- Around line 28-32: The concatenation used to form OFFICE_CONTEXT_DOC_KEY
("doc" + "ument") is unclear to readers; update the code around
OFFICE_CONTEXT_DOC_KEY and getOfficeContextDoc to keep the concatenation but add
a concise comment explaining that the split-string pattern is intentional to
avoid triggering static analyzers or security scanners in Office Add-in contexts
and that it must remain as-is to reliably access Office.context["document"];
include a note for future maintainers stating that if static analysis rules
change this can be replaced with the literal "document".
In `@turbo.json`:
- Around line 4-7: The "build" pipeline currently has "outputs": [] which
prevents Turbo from caching build artifacts; update the "build" pipeline (the
"build" object) to list the actual output globs you want cached (for example
Next.js .next, static out, or package dist folders) so Turbo can store/restore
artifacts instead of only replaying logs—replace the empty array with
appropriate output patterns such as .next/**, out/**, dist/** or
packages/*/dist/** depending on your project outputs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27a2eb0f-caa3-4826-979e-c2ab8e124ee8
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (54)
.gitignoreREADME.mdapps/powerpoint-addin/package.jsonapps/powerpoint-addin/postcss.config.mjsapps/powerpoint-addin/src/App.tsxapps/powerpoint-addin/src/components/ConnectionStatus.tsxapps/powerpoint-addin/src/components/SettingsPanel.tsxapps/powerpoint-addin/src/components/SimulatorPanel.tsxapps/powerpoint-addin/src/index.cssapps/powerpoint-addin/src/lib/officeBridge.tsapps/powerpoint-addin/src/main.tsxapps/powerpoint-addin/src/store/connectionStore.tsapps/powerpoint-addin/src/store/simulatorStore.tsapps/powerpoint-addin/src/store/types.tsapps/powerpoint-addin/src/store/uiStore.tsapps/web/components.jsonapps/web/package.jsonapps/web/postcss.config.mjsapps/web/src/app/api/auth/session/route.tsapps/web/src/app/auth/demo/page.tsxapps/web/src/app/auth/login/page.tsxapps/web/src/app/auth/register/page.tsxapps/web/src/app/dashboard/handout/[id]/print/page.tsxapps/web/src/app/dashboard/layout.tsxapps/web/src/app/dashboard/page.tsxapps/web/src/app/dashboard/session/[id]/page.tsxapps/web/src/app/globals.cssapps/web/src/app/h/[token]/page.tsxapps/web/src/app/layout.tsxapps/web/src/app/powerpoint/addin/page.tsxapps/web/src/components/ConvexClientProvider.tsxapps/web/src/components/ThemeToggle.tsxapps/web/src/components/dashboard/DashboardShell.tsxapps/web/src/components/dashboard/QRCodeDialog.tsxapps/web/src/components/powerpoint/PowerPointAddinClient.tsxapps/web/src/components/ui/Modal.tsxapps/web/src/components/ui/button.tsxapps/web/src/components/ui/card.tsxapps/web/src/components/ui/dialog.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/ui/select.tsxapps/web/src/components/ui/tabs.tsxapps/web/src/components/ui/textarea.tsxapps/web/src/lib/authSession.tsapps/web/src/lib/powerpoint/officeBridge.tsapps/web/src/lib/utils.tsapps/web/src/middleware.tsapps/web/src/store/authStore.tsapps/web/src/store/dashboardStore.tsapps/web/tailwind.config.tsconvex/_utils.tsnetlify.tomlpackage.jsonturbo.json
💤 Files with no reviewable changes (2)
- netlify.toml
- apps/web/src/store/dashboardStore.ts
| const config = { | ||
| plugins: { | ||
| tailwindcss: {}, | ||
| autoprefixer: {}, | ||
| "@tailwindcss/postcss": {}, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- any `@config` directives under apps/web/src ---"
rg -n '@config' apps/web/src || true
echo "--- apps/web/tailwind.config.ts ---"
sed -n '1,120p' apps/web/tailwind.config.ts
echo "--- possible usages of config-only utilities ---"
rg -nP 'animate-fade-in|animate-slide-down|\b(?:bg|text|border|ring)-(brand|emerald)-' apps/web/src || trueRepository: EinfachMxrc/handout
Length of output: 4740
🌐 Web query:
Tailwind CSS v4 PostCSS plugin auto-discovery config file same directory
💡 Result:
No, the Tailwind CSS v4 PostCSS plugin (@tailwindcss/postcss) does not automatically discover a tailwind.config.js (or similar) file in the same directory or anywhere else. JavaScript config files are no longer auto-detected for backward compatibility reasons. To use a JS config file, you must explicitly load it in your CSS using the @config directive, placed after @import "tailwindcss" and before other directives like @theme. Example: @import "tailwindcss"; @config "./tailwind.config.js"; // relative path from the CSS file @theme { // optional CSS config } The primary configuration method in v4 is now CSS-first using @theme, @plugin, etc., directly in the CSS file — no config file needed for most cases. The PostCSS plugin processes the CSS file (including directives) during the build.
Citations:
- 1: https://tailwindcss.com/docs/upgrade-guide
- 2: https://tailwindcss.com/docs/installation/using-postcss
- 3: https://tailwindcss.com/blog/tailwindcss-v4
- 4: https://v3.tailwindcss.com/docs/v4-beta
- 5: https://tailwindcss.com/docs/content-configuration
- 6: Support loading config files via
@configtailwindlabs/tailwindcss#14239 - 7: Inject
@config "..."when atailwind.config.{js,ts,...}is detected tailwindlabs/tailwindcss#14635
Add @config directive to load the Tailwind config in CSS.
Tailwind v4 does not auto-detect JavaScript config files. The existing apps/web/tailwind.config.ts contains custom theme extensions (brand and emerald colors, animations) that are actively used throughout the codebase, but they will silently stop generating without an explicit @config directive.
In apps/web/src/app/globals.css, add the directive immediately after @import "tailwindcss":
`@import` "tailwindcss";
`@config` "./tailwind.config.ts";
Without this, utilities like bg-emerald-*, border-emerald-*, ring-emerald-*, and custom animations will not be generated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/postcss.config.mjs` around lines 1 - 5, The project is missing the
Tailwind `@config` directive so the custom theme in apps/web/tailwind.config.ts
(brand/emerald colors and animations) isn't being loaded; open
apps/web/src/app/globals.css and immediately after the existing `@import`
"tailwindcss" line add an `@config` "./tailwind.config.ts" directive to explicitly
point Tailwind to the JS/TS config so utilities like bg-emerald-*,
border-emerald-*, ring-emerald-*, and custom animations are generated; you can
still keep the postcss config (const config.plugins["@tailwindcss/postcss"])
as-is.
| setAuth(result.token, result.name ?? undefined, result.email, result.isDemo ?? false); | ||
| router.push("/dashboard"); | ||
| await setServerSessionCookie(result.token); | ||
| router.replace("/dashboard"); |
There was a problem hiding this comment.
Persist the server session before mutating client auth state.
Line 39 commits the token locally before Line 40 proves the server session was stored. If /api/auth/session fails, the login page shows an error but the rest of the client can still think the user is authenticated.
Proposed fix
- setAuth(result.token, result.name ?? undefined, result.email, result.isDemo ?? false);
- await setServerSessionCookie(result.token);
+ await setServerSessionCookie(result.token);
+ setAuth(result.token, result.name ?? undefined, result.email, result.isDemo ?? false);
router.replace("/dashboard");📝 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.
| setAuth(result.token, result.name ?? undefined, result.email, result.isDemo ?? false); | |
| router.push("/dashboard"); | |
| await setServerSessionCookie(result.token); | |
| router.replace("/dashboard"); | |
| await setServerSessionCookie(result.token); | |
| setAuth(result.token, result.name ?? undefined, result.email, result.isDemo ?? false); | |
| router.replace("/dashboard"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/auth/login/page.tsx` around lines 39 - 41, The client auth
state is being set via setAuth before ensuring the server session cookie was
successfully persisted with setServerSessionCookie, which can leave the client
incorrectly authenticated if the server call fails; change the call order so you
await setServerSessionCookie(result.token) first, only then call
setAuth(result.token, result.name ?? undefined, result.email, result.isDemo ??
false) and router.replace("/dashboard"); also add error handling around
setServerSessionCookie to avoid mutating client state or to rollback setAuth and
prevent navigation if the server call throws.
| setAuth(result.token as string, name, email, result.isDemo ?? false); | ||
| router.push("/dashboard"); | ||
| await setServerSessionCookie(result.token as string); | ||
| router.replace("/dashboard"); |
There was a problem hiding this comment.
Only mark the client as authenticated after the cookie write succeeds.
Line 45 updates the local auth store before Line 46 awaits the session-cookie request. If that POST fails, this page stays locally authenticated while the next /dashboard navigation is still rejected by the missing cookie.
Suggested fix
- setAuth(result.token as string, name, email, result.isDemo ?? false);
- await setServerSessionCookie(result.token as string);
+ const nextToken = result.token as string;
+ await setServerSessionCookie(nextToken);
+ setAuth(nextToken, name, email, result.isDemo ?? false);
router.replace("/dashboard");📝 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.
| setAuth(result.token as string, name, email, result.isDemo ?? false); | |
| router.push("/dashboard"); | |
| await setServerSessionCookie(result.token as string); | |
| router.replace("/dashboard"); | |
| const nextToken = result.token as string; | |
| await setServerSessionCookie(nextToken); | |
| setAuth(nextToken, name, email, result.isDemo ?? false); | |
| router.replace("/dashboard"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/auth/register/page.tsx` around lines 45 - 47, Move the local
authentication state update and navigation until after the server cookie has
been successfully written: call await setServerSessionCookie(result.token as
string) first, and only if that promise resolves, call setAuth(result.token as
string, name, email, result.isDemo ?? false) and then
router.replace("/dashboard"); if setServerSessionCookie rejects, do not call
setAuth or router.replace — instead surface/handle the error (e.g., show an
error toast) so the UI remains unauthenticated.
| await clearServerSessionCookie(); | ||
| clearAuth(); | ||
| router.replace("/auth/login"); |
There was a problem hiding this comment.
Don't clear local auth until the server cookie delete is confirmed.
clearServerSessionCookie() only awaits fetch() and never checks response.ok in apps/web/src/lib/authSession.ts. If /api/auth/session returns 500, the HttpOnly cookie survives, but Lines 27-28 still clear local state and navigate to /auth/login; the middleware will then redirect that request straight back to /dashboard. Please make the helper fail on non-OK responses and only clearAuth()/redirect after the delete is confirmed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/dashboard/DashboardShell.tsx` around lines 26 - 28,
The current flow calls clearServerSessionCookie() but doesn't verify deletion
succeeded before clearing local state; update clearServerSessionCookie (in
lib/authSession.ts) to check the fetch response.ok and throw or return a failure
on non-OK responses, then in DashboardShell.tsx only call clearAuth() and
router.replace("/auth/login") after clearServerSessionCookie() resolves
successfully; if the helper throws or returns false, handle the error (log/show)
and do not clear local auth or navigate.
| const effectiveMode = useMemo<Mode>(() => { | ||
| if (mode === "system") { | ||
| return resolvedTheme === "dark" ? "dark" : "light"; | ||
| } | ||
| return mode; | ||
| }, [mode, resolvedTheme]); |
There was a problem hiding this comment.
Make the icon branches mutually exclusive.
When mode === "system", effectiveMode still becomes "light" or "dark", so one of these branches renders and the existing system branch on Line 52 renders too. The toggle ends up showing two icons at once in system mode.
🩹 Minimal fix
- {effectiveMode === "light" && (
+ {mode !== "system" && effectiveMode === "light" && (
<svg className="h-5 w-5" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}>
@@
- {effectiveMode === "dark" && (
+ {mode !== "system" && effectiveMode === "dark" && (
<svg className="h-5 w-5" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}>Also applies to: 34-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ThemeToggle.tsx` around lines 17 - 22, The toggle
shows two icons in system mode because effectiveMode currently resolves system
to "light" or "dark"; update the useMemo in ThemeToggle so it preserves "system"
(i.e., return mode when mode === "system") instead of mapping it to
"light"/"dark", so icon-rendering branches become mutually exclusive; locate the
useMemo that computes effectiveMode and change its logic to return mode
('system') when mode === "system" (keeping [mode, resolvedTheme] deps) so only
the system icon branch or the light/dark branch renders.
| function DialogDescription({ | ||
| className, | ||
| ...props | ||
| }: DialogPrimitive.Description.Props) { | ||
| return ( | ||
| <DialogPrimitive.Description | ||
| data-slot="dialog-description" | ||
| className={cn( | ||
| "text-sm text-muted-foreground *:[a]:underline *:[a]:underline-offset-3 *:[a]:hover:text-foreground", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Tailwind CSS v4 arbitrary variant syntax for descendant selectors
💡 Result:
In Tailwind CSS v4, arbitrary variant syntax allows custom selectors using square bracket notation, such as [& *]:bg-red-500 to target direct children or [& ]:bg-blue-500 for deeper descendants. The general form is [selector]:utility, where selector uses & for the parent element. For descendant selectors specifically: - Direct children (child combinator): [& > *]:utility or simply *:utility (built-in child variant). - All descendants (descendant combinator): [& *]:utility. Examples: - [& :hover]:underline — underlines hovered descendants. - [& .child]:bg-green-500 — targets elements with class 'child' anywhere in descendants. - lg:[&:nth-child(-n+3)]:hover:underline — responsive arbitrary variant for first 3 children on hover. This syntax works in v4 as confirmed by official docs and GitHub discussions where arbitrary variants like [&:has([aria-selected])]:text-white are validated. The * built-in variant is for direct children (> ), while arbitrary enables full CSS descendant selectors (). Arbitrary variants support chaining, composability with named variants like data-, and complex pseudo-classes/selectors.
Citations:
- 1: https://tailwindcss.com/blog/tailwindcss-v4
- 2: https://v3.tailwindcss.com/docs/v4-beta
- 3: https://tailwindcss.com/docs/adding-custom-styles
- 4: Arbitrary variants not working after upgrading to TW v4 tailwindlabs/tailwindcss#16599
- 5: [v4] Chaining arbitrary variants with BEM-like partial class name selectors no longer works tailwindlabs/tailwindcss#14844
Use proper Tailwind CSS v4 syntax for descendant selectors.
The *:[a] syntax is not valid in Tailwind CSS v4. To target anchor elements within the description, use the standard arbitrary variant syntax: [&_a]:underline, [&_a]:underline-offset-3, and [&_a:hover]:text-foreground.
Current code
className={cn(
"text-sm text-muted-foreground *:[a]:underline *:[a]:underline-offset-3 *:[a]:hover:text-foreground",
className
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ui/dialog.tsx` around lines 133 - 147, The Tailwind
descendant selectors in DialogDescription are invalid; update the className
passed to DialogPrimitive.Description inside the DialogDescription function to
replace the incorrect "*:[a]..." variants with valid Tailwind v4 arbitrary
variants targeting anchors, e.g. use "[&_a]:underline",
"[&_a]:underline-offset-3", and "[&_a:hover]:text-foreground" (preserve the
existing "text-sm text-muted-foreground" and keep concatenation via cn and the
className prop).
| const sessionToken = request.cookies.get(AUTH_SESSION_COOKIE)?.value; | ||
| const isDashboardPath = pathname.startsWith("/dashboard"); | ||
| const isAuthPath = pathname.startsWith("/auth"); | ||
|
|
||
| if (isDashboardPath && !sessionToken) { | ||
| const url = request.nextUrl.clone(); | ||
| url.pathname = "/auth/login"; | ||
| return NextResponse.redirect(url); | ||
| } | ||
|
|
||
| if (isAuthPath && sessionToken) { | ||
| const url = request.nextUrl.clone(); | ||
| url.pathname = "/dashboard"; | ||
| return NextResponse.redirect(url); |
There was a problem hiding this comment.
Cookie presence alone is not a real auth check.
This middleware treats any AUTH_SESSION_COOKIE value as authenticated state, but the new session API currently stores that cookie for any non-empty token body. A client can mint an arbitrary cookie and satisfy both of these redirects without a validated session. Please validate the token before issuing the cookie, or switch to a signed/opaque server session and verify that here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/middleware.ts` around lines 7 - 20, The middleware currently
treats any non-empty AUTH_SESSION_COOKIE value (sessionToken) as authenticated;
change the logic in middleware.ts to verify the session token before redirect
decisions by replacing the simple presence check with a server-side validation
call (e.g. verifySessionToken(sessionToken) or
getSessionFromToken(sessionToken)) and use the returned session/boolean to
determine isAuthenticated; update the isDashboardPath and isAuthPath branches to
redirect only when isAuthenticated is false (for dashboard) or true (for auth),
or switch to verifying a signed/opaque server session instead of trusting the
raw cookie value.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/web/scripts/dev.mjs (1)
20-27: Fallback port is not verified for availability.If both port 3000 and 3002 are in use, the script will attempt to start on 3002 and fail with an unclear error from Next.js. Consider checking the fallback port as well.
🛠️ Suggested improvement
async function start() { const preferredPort = 3000; const fallbackPort = 3002; - const port = (await isPortFree(preferredPort)) ? preferredPort : fallbackPort; + let port = preferredPort; + if (!(await isPortFree(preferredPort))) { + if (!(await isPortFree(fallbackPort))) { + console.error(`[web] Both ports ${preferredPort} and ${fallbackPort} are in use.`); + process.exit(1); + } + port = fallbackPort; + } if (port !== preferredPort) { console.log(`[web] Port ${preferredPort} is in use, starting on ${fallbackPort}.`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/dev.mjs` around lines 20 - 27, In start(), the fallbackPort (3002) is not verified so if both preferredPort (3000) and fallbackPort are occupied the script will still pick 3002 and Next.js will error; update the logic around isPortFree, preferredPort, fallbackPort and port to check fallbackPort as well (or iterate to find the next free port), set port to the first free port returned by isPortFree, and when no port is available log a clear error (including which ports were tried) and exit gracefully instead of proceeding to start. Ensure references to start(), preferredPort, fallbackPort, isPortFree and port are used so the change is localized to this function.apps/web/src/app/api/auth/demo/route.ts (1)
12-31: Deduplicate Convex URL validation logic into a shared utility.
isLikelyPlaceholder/getConvexUrlhere duplicates logic already present inapps/web/src/lib/convex.ts(Lines 5–20). Consolidating avoids drift and inconsistent acceptance rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/auth/demo/route.ts` around lines 12 - 31, This duplicates the Convex URL validation (isLikelyPlaceholder and getConvexUrl); remove these local implementations and import the shared validation utilities from the existing convex utility module, replacing local calls with the shared getConvexUrl/isLikelyPlaceholder exports (or a single exported function) so all code uses the same validation logic; ensure you update any callers in this file to use the imported symbol(s) and remove the duplicated functions to avoid divergence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/api/auth/demo/route.ts`:
- Around line 50-55: The response currently leaks result.token in the JSON body
(NextResponse.json) while also setting the same token as an httpOnly cookie;
remove the token property from the JSON payload and only return non-sensitive
fields (e.g., name, email, isDemo) so client-side JS cannot read the session
token, and ensure the existing cookie-setting logic that writes the token with
httpOnly=true continues to use result.token for the cookie (reference the
NextResponse.json call that builds response and the cookie-set code that writes
the token).
- Around line 33-65: The POST handler sets an auth cookie without validating
request origin, allowing CSRF/session overwrite; update the POST function to
validate the incoming request origin or same-site header before issuing the
cookie: retrieve the request (e.g., from NextRequest in the POST handler), check
the Origin and/or Referer against an allowlist (or enforce same-site via
checking request.headers.get("origin")/get("referer") and compare to
getConvexUrl() or your app origin), and return a 403/400 response if invalid;
only after passing this validation proceed to call ConvexHttpClient, create the
NextResponse, and set the AUTH_SESSION_COOKIE with existing options (httpOnly,
secure, sameSite, maxAge) to prevent cross-site login.
In `@apps/web/src/lib/convex.ts`:
- Around line 25-27: The code creates convexClient using ConvexReactClient and
falls back to a real deployment URL ("https://gentle-otter-123.convex.cloud")
when hasValidConvexUrl is false; replace that hardcoded external fallback with a
safe local/no-op option (e.g., an empty string, localhost dev URL, or
throw/disable client) so queries/mutations are never routed to an external
deployment; update the convexClient initialization logic around
ConvexReactClient, hasValidConvexUrl, and convexUrl to use the safe fallback and
ensure any callers handle the disabled client appropriately.
In `@package.json`:
- Around line 14-15: The build:legacy npm script uses unscoped package filters
("shared", "reveal-engine") that don't match the workspace package names,
causing filter mismatches; update the build:legacy script (the "build:legacy"
entry in package.json) to use the correct scoped package names
(`@slide-handout/shared` and `@slide-handout/reveal-engine`) so it mirrors the
"build" script's filters and runs pnpm --filter `@slide-handout/shared` build &&
pnpm --filter `@slide-handout/reveal-engine` build (keeping the existing web and
powerpoint-addin filters).
---
Nitpick comments:
In `@apps/web/scripts/dev.mjs`:
- Around line 20-27: In start(), the fallbackPort (3002) is not verified so if
both preferredPort (3000) and fallbackPort are occupied the script will still
pick 3002 and Next.js will error; update the logic around isPortFree,
preferredPort, fallbackPort and port to check fallbackPort as well (or iterate
to find the next free port), set port to the first free port returned by
isPortFree, and when no port is available log a clear error (including which
ports were tried) and exit gracefully instead of proceeding to start. Ensure
references to start(), preferredPort, fallbackPort, isPortFree and port are used
so the change is localized to this function.
In `@apps/web/src/app/api/auth/demo/route.ts`:
- Around line 12-31: This duplicates the Convex URL validation
(isLikelyPlaceholder and getConvexUrl); remove these local implementations and
import the shared validation utilities from the existing convex utility module,
replacing local calls with the shared getConvexUrl/isLikelyPlaceholder exports
(or a single exported function) so all code uses the same validation logic;
ensure you update any callers in this file to use the imported symbol(s) and
remove the duplicated functions to avoid divergence.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69bcbf68-a942-4f50-8782-91c76255e89d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
README.mdapps/powerpoint-addin/package.jsonapps/web/.env.local.exampleapps/web/package.jsonapps/web/scripts/dev.mjsapps/web/src/app/api/auth/demo/route.tsapps/web/src/app/auth/demo/page.tsxapps/web/src/components/ConvexClientProvider.tsxapps/web/src/components/ThemeToggle.tsxapps/web/src/lib/convex.tsconvex/package.jsonpackage.json
✅ Files skipped from review due to trivial changes (2)
- apps/web/.env.local.example
- convex/package.json
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/powerpoint-addin/package.json
- apps/web/src/components/ThemeToggle.tsx
- README.md
- apps/web/package.json
- apps/web/src/app/auth/demo/page.tsx
| export async function POST() { | ||
| const convexUrl = getConvexUrl(); | ||
|
|
||
| if (!convexUrl) { | ||
| return NextResponse.json( | ||
| { error: "NEXT_PUBLIC_CONVEX_URL fehlt oder ist ungueltig." }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| const client = new ConvexHttpClient(convexUrl); | ||
| const result = await client.mutation(api.auth.login, { | ||
| email: DEMO_EMAIL, | ||
| password: DEMO_PASSWORD, | ||
| }); | ||
|
|
||
| const response = NextResponse.json({ | ||
| token: result.token, | ||
| name: result.name ?? "Demo Presenter", | ||
| email: result.email ?? DEMO_EMAIL, | ||
| isDemo: result.isDemo ?? true, | ||
| }); | ||
|
|
||
| response.cookies.set({ | ||
| name: AUTH_SESSION_COOKIE, | ||
| value: result.token, | ||
| httpOnly: true, | ||
| secure: process.env.NODE_ENV === "production", | ||
| sameSite: "lax", | ||
| path: "/", | ||
| maxAge: AUTH_SESSION_MAX_AGE_SECONDS, | ||
| }); |
There was a problem hiding this comment.
Add origin validation to prevent login CSRF/session overwrite.
This endpoint sets an auth cookie but does not validate request origin. A cross-site POST can force a victim into a demo session.
Suggested fix
-export async function POST() {
+export async function POST(request: Request) {
+ const origin = request.headers.get("origin");
+ const requestOrigin = new URL(request.url).origin;
+ if (!origin || origin !== requestOrigin) {
+ return NextResponse.json({ error: "Invalid origin." }, { status: 403 });
+ }
+
const convexUrl = getConvexUrl();📝 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.
| export async function POST() { | |
| const convexUrl = getConvexUrl(); | |
| if (!convexUrl) { | |
| return NextResponse.json( | |
| { error: "NEXT_PUBLIC_CONVEX_URL fehlt oder ist ungueltig." }, | |
| { status: 500 } | |
| ); | |
| } | |
| try { | |
| const client = new ConvexHttpClient(convexUrl); | |
| const result = await client.mutation(api.auth.login, { | |
| email: DEMO_EMAIL, | |
| password: DEMO_PASSWORD, | |
| }); | |
| const response = NextResponse.json({ | |
| token: result.token, | |
| name: result.name ?? "Demo Presenter", | |
| email: result.email ?? DEMO_EMAIL, | |
| isDemo: result.isDemo ?? true, | |
| }); | |
| response.cookies.set({ | |
| name: AUTH_SESSION_COOKIE, | |
| value: result.token, | |
| httpOnly: true, | |
| secure: process.env.NODE_ENV === "production", | |
| sameSite: "lax", | |
| path: "/", | |
| maxAge: AUTH_SESSION_MAX_AGE_SECONDS, | |
| }); | |
| export async function POST(request: Request) { | |
| const origin = request.headers.get("origin"); | |
| const requestOrigin = new URL(request.url).origin; | |
| if (!origin || origin !== requestOrigin) { | |
| return NextResponse.json({ error: "Invalid origin." }, { status: 403 }); | |
| } | |
| const convexUrl = getConvexUrl(); | |
| if (!convexUrl) { | |
| return NextResponse.json( | |
| { error: "NEXT_PUBLIC_CONVEX_URL fehlt oder ist ungueltig." }, | |
| { status: 500 } | |
| ); | |
| } | |
| try { | |
| const client = new ConvexHttpClient(convexUrl); | |
| const result = await client.mutation(api.auth.login, { | |
| email: DEMO_EMAIL, | |
| password: DEMO_PASSWORD, | |
| }); | |
| const response = NextResponse.json({ | |
| token: result.token, | |
| name: result.name ?? "Demo Presenter", | |
| email: result.email ?? DEMO_EMAIL, | |
| isDemo: result.isDemo ?? true, | |
| }); | |
| response.cookies.set({ | |
| name: AUTH_SESSION_COOKIE, | |
| value: result.token, | |
| httpOnly: true, | |
| secure: process.env.NODE_ENV === "production", | |
| sameSite: "lax", | |
| path: "/", | |
| maxAge: AUTH_SESSION_MAX_AGE_SECONDS, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/auth/demo/route.ts` around lines 33 - 65, The POST
handler sets an auth cookie without validating request origin, allowing
CSRF/session overwrite; update the POST function to validate the incoming
request origin or same-site header before issuing the cookie: retrieve the
request (e.g., from NextRequest in the POST handler), check the Origin and/or
Referer against an allowlist (or enforce same-site via checking
request.headers.get("origin")/get("referer") and compare to getConvexUrl() or
your app origin), and return a 403/400 response if invalid; only after passing
this validation proceed to call ConvexHttpClient, create the NextResponse, and
set the AUTH_SESSION_COOKIE with existing options (httpOnly, secure, sameSite,
maxAge) to prevent cross-site login.
| const response = NextResponse.json({ | ||
| token: result.token, | ||
| name: result.name ?? "Demo Presenter", | ||
| email: result.email ?? DEMO_EMAIL, | ||
| isDemo: result.isDemo ?? true, | ||
| }); |
There was a problem hiding this comment.
Do not return the session token in JSON when using HttpOnly cookies.
Line 51 exposes the token to client-side JS, which defeats the security benefit of storing it as httpOnly on Lines 57–60.
Suggested fix
- const response = NextResponse.json({
- token: result.token,
- name: result.name ?? "Demo Presenter",
- email: result.email ?? DEMO_EMAIL,
- isDemo: result.isDemo ?? true,
- });
+ const response = NextResponse.json({
+ name: result.name ?? "Demo Presenter",
+ email: result.email ?? DEMO_EMAIL,
+ isDemo: result.isDemo ?? true,
+ });📝 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.
| const response = NextResponse.json({ | |
| token: result.token, | |
| name: result.name ?? "Demo Presenter", | |
| email: result.email ?? DEMO_EMAIL, | |
| isDemo: result.isDemo ?? true, | |
| }); | |
| const response = NextResponse.json({ | |
| name: result.name ?? "Demo Presenter", | |
| email: result.email ?? DEMO_EMAIL, | |
| isDemo: result.isDemo ?? true, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/auth/demo/route.ts` around lines 50 - 55, The response
currently leaks result.token in the JSON body (NextResponse.json) while also
setting the same token as an httpOnly cookie; remove the token property from the
JSON payload and only return non-sensitive fields (e.g., name, email, isDemo) so
client-side JS cannot read the session token, and ensure the existing
cookie-setting logic that writes the token with httpOnly=true continues to use
result.token for the cookie (reference the NextResponse.json call that builds
response and the cookie-set code that writes the token).
| export const convexClient = new ConvexReactClient( | ||
| convexUrl ?? "https://placeholder.convex.cloud" | ||
| hasValidConvexUrl ? convexUrl : "https://gentle-otter-123.convex.cloud" | ||
| ); |
There was a problem hiding this comment.
Avoid using a real Convex deployment as fallback.
On Line 26, falling back to https://gentle-otter-123.convex.cloud can route unintended queries/mutations to an external deployment when env config is invalid. That creates avoidable data leakage risk.
Suggested fix
export const convexClient = new ConvexReactClient(
- hasValidConvexUrl ? convexUrl : "https://gentle-otter-123.convex.cloud"
+ hasValidConvexUrl ? convexUrl : "https://convex.invalid"
);📝 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.
| export const convexClient = new ConvexReactClient( | |
| convexUrl ?? "https://placeholder.convex.cloud" | |
| hasValidConvexUrl ? convexUrl : "https://gentle-otter-123.convex.cloud" | |
| ); | |
| export const convexClient = new ConvexReactClient( | |
| hasValidConvexUrl ? convexUrl : "https://convex.invalid" | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/convex.ts` around lines 25 - 27, The code creates
convexClient using ConvexReactClient and falls back to a real deployment URL
("https://gentle-otter-123.convex.cloud") when hasValidConvexUrl is false;
replace that hardcoded external fallback with a safe local/no-op option (e.g.,
an empty string, localhost dev URL, or throw/disable client) so
queries/mutations are never routed to an external deployment; update the
convexClient initialization logic around ConvexReactClient, hasValidConvexUrl,
and convexUrl to use the safe fallback and ensure any callers handle the
disabled client appropriately.
| "build": "turbo run build --filter=@slide-handout/shared --filter=@slide-handout/reveal-engine --filter=web --filter=powerpoint-addin", | ||
| "build:legacy": "pnpm --filter reveal-engine build && pnpm --filter shared build && pnpm --filter web build && pnpm --filter powerpoint-addin build", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify package names in each workspace to confirm filter consistency
echo "=== Checking package names in workspaces ==="
for pkg in packages/shared packages/reveal-engine apps/web apps/powerpoint-addin; do
if [ -f "$pkg/package.json" ]; then
name=$(cat "$pkg/package.json" | grep -m1 '"name"' | sed 's/.*: *"\([^"]*\)".*/\1/')
echo "$pkg -> $name"
fi
doneRepository: EinfachMxrc/handout
Length of output: 261
Fix inconsistent package filter names in build:legacy script.
The build script correctly uses scoped package names (@slide-handout/shared, @slide-handout/reveal-engine), but build:legacy uses bare names (reveal-engine, shared) that don't match the actual package names defined in the workspace. This will likely cause build failures or unexpected behavior. Update build:legacy to use the correct scoped package names:
"build:legacy": "pnpm --filter `@slide-handout/shared` build && pnpm --filter `@slide-handout/reveal-engine` build && pnpm --filter web build && pnpm --filter powerpoint-addin build"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 14 - 15, The build:legacy npm script uses unscoped
package filters ("shared", "reveal-engine") that don't match the workspace
package names, causing filter mismatches; update the build:legacy script (the
"build:legacy" entry in package.json) to use the correct scoped package names
(`@slide-handout/shared` and `@slide-handout/reveal-engine`) so it mirrors the
"build" script's filters and runs pnpm --filter `@slide-handout/shared` build &&
pnpm --filter `@slide-handout/reveal-engine` build (keeping the existing web and
powerpoint-addin filters).
Summary by CodeRabbit
New Features
Improvements
Bug Fixes