Migrate site to Vite + React 19 + Tailwind v4#17
Conversation
…esign language - Replace Tailwind-CDN static HTML pages with a React SPA (React Router 7) built by Vite with @cloudflare/vite-plugin; worker moved to worker/ - All 12 pages ported as routes with copy, images, and behavior preserved (WebSerial flasher, font builder, theme builder, debug console, admin, insider/login, kosync, sticky, unlocker, docs, roadmap) - Shared design system: brand-green tokens on Free-Ink patterns (Space Grotesk display, IBM Plex Mono labels, dot-field/blueprint textures), shared Header/Footer/Layout/Modal components - Legacy .html URLs 301-redirect to SPA routes; SPA fallback enabled
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
crosspoint-tools | a043c0f | Jul 14 2026, 02:22 PM |
📝 WalkthroughWalkthroughThe PR migrates CrossPoint Reader to a Vite-powered React SPA with routed pages, shared layout components, device flashing tools, documentation and font builders, an admin dashboard, and updated Cloudflare Worker asset handling. Legacy static HTML pages and JavaScript modules are removed. ChangesReact SPA migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
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 |
What to change in Workers & Pages → crosspoint-tools → Settings → Builds: Build command: npm run build - Deploy command: leave as npx wrangler deploy (it finds the generated config via the redirect file) |
Introduce short labels for navigation items that display on medium viewports, with full labels shown on extra-large screens. Add NavLabel component to handle label switching and make FundButton text responsive ("Fund" on mobile, "Fund CrossPoint" on larger screens). Adjust spacing and add whitespace-nowrap to prevent text wrapping.
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (9)
src/pages/LoginPage.jsx (1)
49-75: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the magic-link fetch.
handleSubmithas no timeout/AbortControlleraround the fetch; a hung request leavessendingtrue indefinitely and the submit button permanently disabled with no feedback to the user.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/LoginPage.jsx` around lines 49 - 75, Update handleSubmit’s magic-link fetch to use an AbortController with a timeout, aborting requests that exceed the chosen limit and surfacing the existing failure status to the user. Ensure cleanup clears the timeout and setSending(false) runs even when the request hangs or is aborted.src/components/SiteBanner.jsx (1)
5-17: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winReplace
dangerouslySetInnerHTMLwith real React elements.The escape-then-linkify ordering currently prevents attribute-breakout and
javascript:URLs, but it's a fragile, hand-rolled pattern flagged by static analysis for both the manual entity-escaping and thedangerouslySetInnerHTMLsink. A future edit to this parsing logic could easily reopen an XSS hole with no compiler/test safety net. Since the only need is to render plain text plus a handful of<a>links, build an array of React nodes instead of an HTML string, removing the sink entirely.♻️ Proposed refactor
-function escapeHtml(s) { - return String(s).replace(/[&<>"']/g, (c) => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": '&`#39`;' }[c])) -} - -// Minimal markdown: [label](url) links plus bare https:// URLs. -function renderBannerHtml(text) { - let html = escapeHtml(text) - html = html.replace(/\[([^\]]+)\]\((https?:\/\/[^\s)]+|\/[^\s)]*)\)/g, - (_, label, url) => `<a href="${url}" class="${LINK_CLASS}">${label}</a>`) - html = html.replace(/(^|\s)(https?:\/\/[^\s<]+)/g, - (_, pre, url) => `${pre}<a href="${url}" class="${LINK_CLASS}">${url}</a>`) - return html -} +// Minimal markdown: [label](url) links plus bare https:// URLs, returned as React nodes. +function renderBannerNodes(text) { + const linkRe = /\[([^\]]+)\]\((https?:\/\/[^\s)]+|\/[^\s)]*)\)|(^|\s)(https?:\/\/[^\s<]+)/g + const nodes = [] + let last = 0 + let match + while ((match = linkRe.exec(text))) { + if (match.index > last) nodes.push(text.slice(last, match.index)) + if (match[1]) { + nodes.push(<a key={nodes.length} href={match[2]} className={LINK_CLASS}>{match[1]}</a>) + } else { + if (match[3]) nodes.push(match[3]) + nodes.push(<a key={nodes.length} href={match[4]} className={LINK_CLASS}>{match[4]}</a>) + } + last = linkRe.lastIndex + } + if (last < text.length) nodes.push(text.slice(last)) + return nodes +}- const [html, setHtml] = useState('') + const [nodes, setNodes] = useState(null) useEffect(() => { let cancelled = false fetch('/api/banner') .then((res) => (res.ok ? res.json() : null)) .then((banner) => { - if (!cancelled && banner?.enabled && banner?.text) setHtml(renderBannerHtml(banner.text)) + if (!cancelled && banner?.enabled && banner?.text) setNodes(renderBannerNodes(banner.text)) }) .catch(() => {}) return () => { cancelled = true } }, []) - if (!html) return null + if (!nodes) return null return ( - <div - className="bg-brand-600 px-6 py-2.5 text-center text-sm font-medium text-white" - dangerouslySetInnerHTML={{ __html: html }} - /> + <div className="bg-brand-600 px-6 py-2.5 text-center text-sm font-medium text-white"> + {nodes} + </div> )Also applies to: 38-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/SiteBanner.jsx` around lines 5 - 17, Refactor renderBannerHtml in SiteBanner.jsx to return React nodes rather than an escaped HTML string, and replace the associated dangerouslySetInnerHTML usage with direct child rendering. Preserve support for markdown-style links and bare https:// URLs, allow only http(s) or relative link targets, and render unmatched content as plain text without manual entity escaping.Source: Linters/SAST tools
package.json (1)
22-30: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
vitepinned to a version now two majors behind and past its security-patch window.Vite has since released v7 (June 2025) and v8 (~Dec 2025/Jan 2026); per Vite's own support policy only the latest two majors receive security patches, so
^6.0.0is now unsupported for patches. Worth bumping and confirming@cloudflare/vite-plugin@^1.40.0's supported Vite range before upgrading.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 22 - 30, Update the devDependency version for vite from ^6.0.0 to a currently supported major, and verify that `@cloudflare/vite-plugin` remains compatible with the selected Vite version before finalizing the dependency change.src/index.css (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStylelint config doesn't recognize Tailwind v4's
@theme/@utilityat-rules.These are correct Tailwind v4 CSS-first directives, but the current stylelint setup (SCSS-oriented
at-rule-no-unknown) flags them as unknown, producing recurring false-positive noise. Worth adding a Tailwind-aware stylelint config/allowlist for these at-rules.Also applies to: 35-60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.css` at line 4, Update the Stylelint configuration’s at-rule validation to recognize Tailwind v4 CSS-first directives, specifically `@theme` and `@utility`, so these rules are not reported as unknown. Preserve the existing SCSS at-rule checks and add the Tailwind directives through the project’s established configuration or allowlist mechanism.Source: Linters/SAST tools
src/pages/fonts/fontBuilder.js (1)
120-137: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider bundling JSZip via npm instead of runtime CDN injection.
loadJSZipinjects a<script>tag pointing at jsDelivr with nointegrity/crossoriginattribute. Now that the project has real npm/Vite bundling, importingjszipstatically avoids the unpinned third-party script-injection trust boundary (and any future CSP friction) entirely.♻️ Suggested approach
-let jszipPromise = null -export function loadJSZip() { - if (typeof window !== 'undefined' && window.JSZip) return Promise.resolve(window.JSZip) - if (!jszipPromise) { - jszipPromise = new Promise((resolve) => { - const s = document.createElement('script') - s.src = 'https://cdn.jsdelivr.net/npm/jszip@3.10.1/dist/jszip.min.js' - s.defer = true - s.onload = () => resolve(window.JSZip || null) - s.onerror = () => resolve(null) - document.head.appendChild(s) - }) - } - return jszipPromise -} +import JSZip from 'jszip' + +export function loadJSZip() { + return Promise.resolve(JSZip) +}Requires adding
"jszip": "^3.10.1"topackage.jsondependencies (the caller inFontsPage.jsxalready falls back to individual downloads ifloadJSZip()resolves falsy, so the fallback path stays intact if you'd rather keep the graceful-degradation behavior with a try/catch around the static import instead).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/fonts/fontBuilder.js` around lines 120 - 137, Replace the runtime CDN script injection in loadJSZip with the project-bundled jszip dependency, adding jszip at the required version and importing it through the existing Vite module flow. Preserve loadJSZip’s promise-based API and its caller’s graceful fallback behavior, including resolving a falsy value if the bundled module cannot be loaded.src/components/Modal.jsx (1)
4-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winModal lacks initial focus and
aria-labelledby.No focus is moved into the dialog on open (keyboard users can Tab straight through into background content) and the dialog isn't programmatically associated with its title for screen readers.
♻️ Proposed fix
-import { useEffect } from 'react' +import { useEffect, useRef } from 'react' export default function Modal({ open, onClose, title, children }) { + const dialogRef = useRef(null) useEffect(() => { if (!open) return + dialogRef.current?.focus() const onKey = (e) => { if (e.key === 'Escape') onClose() } ... }, [open, onClose]) if (!open) return null return ( <div className="fixed inset-0 z-[100] flex items-center justify-center px-4 py-8" role="dialog" aria-modal="true"> <div className="absolute inset-0 bg-stone-900/40 backdrop-blur-sm" onClick={onClose} /> - <div className="relative max-h-[calc(100vh-4rem)] w-full max-w-lg overflow-y-auto rounded-2xl bg-white shadow-xl ring-1 ring-stone-950/5"> + <div + ref={dialogRef} + tabIndex={-1} + aria-labelledby="modal-title" + className="relative max-h-[calc(100vh-4rem)] w-full max-w-lg overflow-y-auto rounded-2xl bg-white shadow-xl ring-1 ring-stone-950/5" + > <div className="flex items-start justify-between gap-4 border-b border-stone-100 px-6 py-4"> - <h2 className="font-display text-xl font-semibold tracking-tight text-stone-900">{title}</h2> + <h2 id="modal-title" className="font-display text-xl font-semibold tracking-tight text-stone-900">{title}</h2>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Modal.jsx` around lines 4 - 38, Update the Modal component to move focus into the dialog when it opens, using a ref on the dialog container and focusing it after render while preserving normal cleanup behavior. Add a stable id to the title rendered by the h2 and set the dialog’s aria-labelledby to that id so assistive technologies associate the dialog with its title.src/pages/AdminPage.jsx (1)
156-209: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdopt
readJsonResponse/describeFailureconsistently instead of rawres.json().These helpers (already imported at line 5) were specifically written to tolerate non-JSON error bodies (edge/WAF challenge pages, per the comment in
admin/api.js), but onlyStickyCarduses them.TriggerBuildCard.triggerBuild,BannerCard.saveBanner, andBetaCard.uploadBeta/saveBetaEditstill callres.json()directly (or.catch(() => ({}))), which surfaces a generic "Connection error" or a raw parse-error message instead of the more informativedescribeFailureoutput when the server returns a non-JSON error page.♻️ Example fix for TriggerBuildCard
const res = await fetch('/api/build/trigger', { method: 'POST', headers: { Authorization: `Bearer ${secret}` }, }) - const data = await res.json() + const r = await readJsonResponse(res) - if (res.ok) { - setResult({ kind: 'ok', text: data.commit }) - log(`Build triggered: ${data.commit}`) + if (r.ok) { + setResult({ kind: 'ok', text: r.data?.commit }) + log(`Build triggered: ${r.data?.commit}`) setTimeout(() => refreshRef.current?.(), 3000) } else { - setResult({ kind: 'error', text: data.error }) - log(`Trigger failed: ${data.error}`) + setResult({ kind: 'error', text: describeFailure(r) }) + log(`Trigger failed: ${describeFailure(r)}`) }Also applies to: 213-263, 336-392, 429-469
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AdminPage.jsx` around lines 156 - 209, Update TriggerBuildCard.triggerBuild, BannerCard.saveBanner, and BetaCard.uploadBeta/saveBetaEdit to use the imported readJsonResponse and describeFailure helpers instead of direct res.json() calls or parse-error fallbacks. Route non-OK responses through describeFailure so non-JSON server bodies produce the informative error text, while preserving existing success handling and UI state updates.src/pages/InsiderPage.jsx (1)
126-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace blocking
alert()validation with the page's existing inline banners.
handleFlashNightly/handleCfFlash(and the catch blocks at lines 346 and 420) use nativealert()for a very reachable user mistake (no device model selected, or a flasher error), while the rest of the page already has styled inline error/warning banners.alert()is jarring, blocks the JS thread, and is inconsistent with the rest of the UX. Consider disabling the flash buttons untildeviceModelis set, or rendering an inline message instead.Also applies to: 363-375
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/InsiderPage.jsx` around lines 126 - 138, Replace the native alert calls in handleFlashNightly, handleCfFlash, and the referenced flasher-error catch blocks with the page’s existing inline error or warning banner state. Render the no-device-model validation and non-NotFoundError messages through that banner mechanism, and preserve the existing early returns and silent handling of NotFoundError.src/pages/ThemeBuilderPage.jsx (1)
19-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd unmount cleanup for the theme builder Return a teardown from
useEffectto clear the icon-build polling timer; leaving the page mid-build can keep background polling/fetches alive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ThemeBuilderPage.jsx` around lines 19 - 23, Update the useEffect that calls initThemeBuilder to return an unmount cleanup function that invokes the theme builder’s existing teardown mechanism for the icon-build polling timer and related fetch activity. Preserve the document title assignment and one-time initialization behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.jsx`:
- Around line 35-48: Update ScrollManager so hash navigation retries locating
the element after lazy-loaded route content mounts instead of immediately
falling back to window.scrollTo(0, 0) when getElementById returns null. Use a
bounded or cancellable retry mechanism, clean it up when pathname or hash
changes/unmounts, and preserve immediate scrolling when the target already
exists.
In `@src/components/DownloadModal.jsx`:
- Around line 48-69: Update the catalog-fetch error path in the useEffect for
DownloadModal so failed requests do not leave catalog set to a truthy value that
blocks future retries. Preserve setLoadError(err.message) and ensure closing and
reopening the modal can trigger /api/catalog again after a transient failure.
In `@src/lib/esptool.bundle.js`:
- Around line 7082-7092: In the detecting branch of detectChip(), validate the
local chip result returned by magic2Chip(chipMagicValue) before assigning it to
this.chip. Throw the existing ESPError for a null/unsupported result, and only
set this.chip when chip is valid.
In `@src/lib/theme-builder.js`:
- Around line 2294-2303: Update initThemeBuilder and the icon-build polling flow
around pollIconBuild so unmounting the theme-builder page cancels any in-flight
polling and prevents further /api/theme-build/status requests. Return or expose
a cleanup function that clears iconPollTimer and invokes it from the page
teardown path, while preserving the existing per-canvas initialization guard.
In `@src/pages/AdminPage.jsx`:
- Line 1008: Remove the client-side persistence of the admin secret in the
AdminPage session-storage flow, specifically the sessionStorage.setItem call
using STORAGE_KEY. Replace this bearer-token storage with an HttpOnly, Secure,
SameSite cookie issued and managed by the worker, updating the associated
authentication flow to use that cookie without exposing the secret to page
JavaScript.
- Around line 980-1013: Update authenticate() to reject every non-2xx response
from /api/build/trigger before calling setSecret or setAuthed, while preserving
the existing “Invalid secret” message for HTTP 401. For other failed statuses,
set an appropriate connection/request error, clear auth busy state, and return;
only allow the existing authenticated flow when res.ok is true.
In `@src/pages/DocsPage.jsx`:
- Around line 15-44: Add an afterSanitizeAttributes hook to the marked
configuration in DocsPage.jsx that detects anchor elements with target="_blank"
and guarantees rel includes both noopener and noreferrer, while preserving any
existing rel values. Apply this to the HTML sanitization flow used for rendered
docs content.
In `@src/pages/FontsPage.jsx`:
- Around line 66-71: Remove the familyNameRef declaration and the unconditional
familyNameRef.current assignment from the FontsPage component; update
downloadAll and any other references to use the current familyName value from
its render closure instead, including the additional usage noted near lines
328-336.
- Around line 407-647: Add htmlFor attributes to every file-input label in the
FontsPage JSX, matching each label to its corresponding input id: fontFolder,
fontRegular, fontBold, fontItalic, fontBoldItalic, fallbackFontFolder,
fallbackFontRegular, fallback2FontFolder, and fallback2FontRegular. Preserve the
existing labels and input behavior.
In `@src/pages/home/FlashTools.jsx`:
- Around line 286-334: Move the setRestart call from the initial setup into the
successful completion path after flasher.flashFirmware resolves. Keep it unset
or false while firmware fetching/flashing is in progress and when the try block
fails, and preserve the existing unplug condition based on skipReset and model
!== 'x4'.
In `@src/pages/home/markdown.js`:
- Around line 14-18: Update safeUrl so the root-relative URL branch accepts a
single leading slash but rejects protocol-relative values beginning with “//”.
Preserve the existing handling for http(s), hash, and mailto URLs, and continue
returning “#” for disallowed inputs.
In `@src/pages/insider/buildShared.jsx`:
- Around line 30-35: Update cleanCommitMessage to handle nullish or missing
message values before calling split, returning a safe empty or established
fallback string while preserving the current formatting behavior for valid
messages.
In `@src/pages/InsiderPage.jsx`:
- Around line 205-231: Re-check cfPollingRef.current immediately after each
fetchCustomBuildStatus() await in both poll() and the visibilitychange handler,
returning without applying results when polling was cleared during the request.
Ensure stale responses cannot call showCustomBuildReady, setCfError, or
setCfState after clearCustomBuild().
- Around line 88-94: Update loadBuildInfo in InsiderPage to wrap fetchBuildMeta
and the success state updates in try/catch/finally, ensuring setLoading(false)
always executes when the request rejects so the refresh UI remains available.
Preserve the existing metadata reset and successful setMeta behavior, and follow
the sibling effect’s established error-handling approach where applicable.
- Around line 733-752: Update the size input handler in the sizes.map callback
to prevent empty or invalid values from storing NaN in cfSizes. Parse the
entered value and retain the previous valid size, or apply an appropriate
existing fallback, when parsing fails; preserve valid numeric updates so
handleCfBuild receives only usable font sizes.
In `@src/pages/LoginPage.jsx`:
- Around line 41-47: Update toggleLogin so it derives the next visibility value
from the current showLogin state, schedules the emailRef focus outside the
setShowLogin updater when opening the login view, and then applies the next
value without side effects inside the updater.
In `@src/pages/StickyPage.jsx`:
- Around line 16-51: Update safeUrl() to reject protocol-relative URLs beginning
with //, and add attribute-safe escaping for URLs before interpolating them into
href values in renderMarkdown(). Apply this to both markdown links and bare URL
links, preserving the existing allowed schemes and rendering behavior; reuse the
established marked/DOMPurify pipeline only if it is already available for this
page.
In `@worker/types.ts`:
- Line 9: Restrict ALLOW_INSECURE_DEV_WEBHOOKS to development or preview
environments in isAuthorizedWebhookRequest, ensuring production cannot disable
webhook signature validation. Alternatively, remove this variable from
production bindings while preserving its intended dev-only behavior.
In `@worker/webhook.ts`:
- Around line 3-25: Update verifyGitHubSignature to use crypto.subtle.verify()
with the imported HMAC key and decoded x-hub-signature-256 value instead of
constructing expected and comparing with ===. Preserve the existing sha256=
prefix validation and return the request body alongside the verification result.
---
Nitpick comments:
In `@package.json`:
- Around line 22-30: Update the devDependency version for vite from ^6.0.0 to a
currently supported major, and verify that `@cloudflare/vite-plugin` remains
compatible with the selected Vite version before finalizing the dependency
change.
In `@src/components/Modal.jsx`:
- Around line 4-38: Update the Modal component to move focus into the dialog
when it opens, using a ref on the dialog container and focusing it after render
while preserving normal cleanup behavior. Add a stable id to the title rendered
by the h2 and set the dialog’s aria-labelledby to that id so assistive
technologies associate the dialog with its title.
In `@src/components/SiteBanner.jsx`:
- Around line 5-17: Refactor renderBannerHtml in SiteBanner.jsx to return React
nodes rather than an escaped HTML string, and replace the associated
dangerouslySetInnerHTML usage with direct child rendering. Preserve support for
markdown-style links and bare https:// URLs, allow only http(s) or relative link
targets, and render unmatched content as plain text without manual entity
escaping.
In `@src/index.css`:
- Line 4: Update the Stylelint configuration’s at-rule validation to recognize
Tailwind v4 CSS-first directives, specifically `@theme` and `@utility`, so these
rules are not reported as unknown. Preserve the existing SCSS at-rule checks and
add the Tailwind directives through the project’s established configuration or
allowlist mechanism.
In `@src/pages/AdminPage.jsx`:
- Around line 156-209: Update TriggerBuildCard.triggerBuild,
BannerCard.saveBanner, and BetaCard.uploadBeta/saveBetaEdit to use the imported
readJsonResponse and describeFailure helpers instead of direct res.json() calls
or parse-error fallbacks. Route non-OK responses through describeFailure so
non-JSON server bodies produce the informative error text, while preserving
existing success handling and UI state updates.
In `@src/pages/fonts/fontBuilder.js`:
- Around line 120-137: Replace the runtime CDN script injection in loadJSZip
with the project-bundled jszip dependency, adding jszip at the required version
and importing it through the existing Vite module flow. Preserve loadJSZip’s
promise-based API and its caller’s graceful fallback behavior, including
resolving a falsy value if the bundled module cannot be loaded.
In `@src/pages/InsiderPage.jsx`:
- Around line 126-138: Replace the native alert calls in handleFlashNightly,
handleCfFlash, and the referenced flasher-error catch blocks with the page’s
existing inline error or warning banner state. Render the no-device-model
validation and non-NotFoundError messages through that banner mechanism, and
preserve the existing early returns and silent handling of NotFoundError.
In `@src/pages/LoginPage.jsx`:
- Around line 49-75: Update handleSubmit’s magic-link fetch to use an
AbortController with a timeout, aborting requests that exceed the chosen limit
and surfacing the existing failure status to the user. Ensure cleanup clears the
timeout and setSending(false) runs even when the request hangs or is aborted.
In `@src/pages/ThemeBuilderPage.jsx`:
- Around line 19-23: Update the useEffect that calls initThemeBuilder to return
an unmount cleanup function that invokes the theme builder’s existing teardown
mechanism for the icon-build polling timer and related fetch activity. Preserve
the document title assignment and one-time initialization behavior.
🪄 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: CHILL
Plan: Pro
Run ID: df1c3835-82db-42f4-87b5-9e12820539a8
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/screenshots/hero-home.pngis excluded by!**/*.png
📒 Files selected for processing (63)
.gitignoreindex.htmlpackage.jsonpublic/admin.htmlpublic/debug.htmlpublic/docs.htmlpublic/fonts.htmlpublic/index.htmlpublic/insider.htmlpublic/js/buy-modal.jspublic/js/download-modal.jspublic/js/site-nav.jspublic/kosync.htmlpublic/login.htmlpublic/roadmap.htmlpublic/sticky.htmlpublic/theme-builder.htmlpublic/unlocker.htmlscripts/test-theme-builder.cjssrc/App.jsxsrc/components/BuyModal.jsxsrc/components/DownloadModal.jsxsrc/components/Footer.jsxsrc/components/Header.jsxsrc/components/Layout.jsxsrc/components/Modal.jsxsrc/components/SiteBanner.jsxsrc/components/ui.jsxsrc/index.csssrc/lib/esptool.bundle.jssrc/lib/flasher.jssrc/lib/theme-builder.jssrc/main.jsxsrc/pages/AdminPage.jsxsrc/pages/DebugPage.jsxsrc/pages/DocsPage.jsxsrc/pages/FontsPage.jsxsrc/pages/HomePage.jsxsrc/pages/InsiderPage.jsxsrc/pages/KosyncPage.jsxsrc/pages/LoginPage.jsxsrc/pages/RoadmapPage.jsxsrc/pages/StickyPage.jsxsrc/pages/ThemeBuilderPage.jsxsrc/pages/UnlockerPage.jsxsrc/pages/admin/api.jssrc/pages/debug/helpers.jssrc/pages/docs/docs-prose.csssrc/pages/fonts/fontBuilder.jssrc/pages/home/Community.jsxsrc/pages/home/Features.jsxsrc/pages/home/FlashTools.jsxsrc/pages/home/GetInTouch.jsxsrc/pages/home/Hero.jsxsrc/pages/home/UnlockSection.jsxsrc/pages/home/markdown.jssrc/pages/insider/buildShared.jsxtsconfig.jsonvite.config.jsworker/index.tsworker/types.tsworker/webhook.tswrangler.jsonc
💤 Files with no reviewable changes (15)
- public/js/site-nav.js
- public/sticky.html
- public/theme-builder.html
- public/js/buy-modal.js
- public/index.html
- public/js/download-modal.js
- public/unlocker.html
- public/docs.html
- public/roadmap.html
- public/kosync.html
- public/debug.html
- public/login.html
- public/insider.html
- public/fonts.html
- public/admin.html
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (19)
src/App.jsx (1)
35-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Hash-anchor scroll races with lazy-loaded routes.
ScrollManagerlives outside the<Suspense>boundary, so its effect runs as soon as the route changes, not once the lazy page has actually mounted. On a first visit to a lazy route with a hash (e.g./roadmap#scope, or#unlock-toolon the home page while it's still loading),document.getElementById(hash.slice(1))returnsnullbecause the target isn't in the DOM yet, and the code silently falls back towindow.scrollTo(0, 0)— defeating the very behavior the comment above it promises.🩹 Proposed fix: retry until the target mounts
function ScrollManager() { const { pathname, hash } = useLocation() useEffect(() => { if (hash) { - const el = document.getElementById(hash.slice(1)) - if (el) { - el.scrollIntoView() - return - } + const id = hash.slice(1) + let attempts = 0 + const tryScroll = () => { + const el = document.getElementById(id) + if (el) { + el.scrollIntoView() + } else if (attempts++ < 30) { + requestAnimationFrame(tryScroll) + } + } + tryScroll() + return } window.scrollTo(0, 0) }, [pathname, hash]) return null }📝 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.function ScrollManager() { const { pathname, hash } = useLocation() useEffect(() => { if (hash) { const id = hash.slice(1) let attempts = 0 const tryScroll = () => { const el = document.getElementById(id) if (el) { el.scrollIntoView() } else if (attempts++ < 30) { requestAnimationFrame(tryScroll) } } tryScroll() return } window.scrollTo(0, 0) }, [pathname, hash]) return null }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.jsx` around lines 35 - 48, Update ScrollManager so hash navigation retries locating the element after lazy-loaded route content mounts instead of immediately falling back to window.scrollTo(0, 0) when getElementById returns null. Use a bounded or cancellable retry mechanism, clean it up when pathname or hash changes/unmounts, and preserve immediate scrolling when the target already exists.src/components/DownloadModal.jsx (1)
48-69: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Failed catalog fetch permanently blocks retries for the session.
On error,
catalogis set to{ releases: [] }(truthy), which forever satisfies theif (!open || catalog) returnguard — closing and reopening the modal never retries the fetch. A single transient network blip permanently degrades the download feature until a full page reload.🔧 Proposed fix
+ // Allow a fresh attempt the next time the modal is reopened after a failed load. + useEffect(() => { + if (!open && loadError) { + setCatalog(null) + setLoadError(null) + } + }, [open, loadError]) + useEffect(() => { if (!open || catalog) return let cancelled = false fetch('/api/catalog')📝 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.// Allow a fresh attempt the next time the modal is reopened after a failed load. useEffect(() => { if (!open && loadError) { setCatalog(null) setLoadError(null) } }, [open, loadError]) useEffect(() => { if (!open || catalog) return let cancelled = false fetch('/api/catalog') .then((res) => { if (!res.ok) throw new Error(`Catalog request failed: ${res.status}`) return res.json() }) .then((data) => { if (!cancelled) setCatalog(data) }) .catch((err) => { console.error(err) if (!cancelled) { setCatalog({ releases: [] }) setLoadError(err.message) } }) return () => { cancelled = true } }, [open, catalog])🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 56-56: Avoid using the initial state variable in setState
Context: setCatalog(data)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DownloadModal.jsx` around lines 48 - 69, Update the catalog-fetch error path in the useEffect for DownloadModal so failed requests do not leave catalog set to a truthy value that blocks future retries. Preserve setLoadError(err.message) and ensure closing and reopening the modal can trigger /api/catalog again after a transient failure.src/lib/esptool.bundle.js (1)
7082-7092: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n "esptool-js" package.jsonRepository: crosspoint-reader/crosspoint-tools
Length of output: 172
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant section and nearby definitions. sed -n '7068,7100p' src/lib/esptool.bundle.js printf '\n---\n' # Look for magic2Chip and chip initialization paths in the bundle. rg -n "magic2Chip|this\.chip === null|this\.chip = chip|CHIP_DETECT_MAGIC_REG_ADDR" src/lib/esptool.bundle.js printf '\n---\n' # Look for any vendoring metadata in the repo. git ls-files | rg '(^|/)(package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock|LICENSE|NOTICE|README)'Repository: crosspoint-reader/crosspoint-tools
Length of output: 2150
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find where the chip field is initialized and how connect() reaches this branch. rg -n "this\.chip\s*=|class .*EspTool|constructor\(" src/lib/esptool.bundle.js | head -n 80 printf '\n---\n' # Print the connect() prologue and a bit of surrounding context. sed -n '7000,7088p' src/lib/esptool.bundle.jsRepository: crosspoint-reader/crosspoint-tools
Length of output: 4223
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the constructor around the chip field initialization. sed -n '6600,6665p' src/lib/esptool.bundle.js printf '\n---\n' # Show the chip-mapping helper. sed -n '6535,6575p' src/lib/esptool.bundle.jsRepository: crosspoint-reader/crosspoint-tools
Length of output: 3492
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find every chip field reference in the bundle. rg -n "this\.chip\b" src/lib/esptool.bundle.js printf '\n---\n' # Show the detectChip logic after connect(). sed -n '7090,7135p' src/lib/esptool.bundle.jsRepository: crosspoint-reader/crosspoint-tools
Length of output: 3726
Check the detected
chipvalue here
this.chipis still unset on the first connect, so this guard never throws for an unsupported magic value. Checkchip === nullbefore assigning it; otherwisedetectChip()falls through tounknown!and later dereferencesthis.chip.Proposed fix
const chip = await magic2Chip(chipMagicValue); - if (this.chip === null) { + if (chip === null) { throw new ESPError(`Unexpected CHIP magic value ${chipMagicValue}. Failed to autodetect chip type.`); } else { this.chip = chip; }📝 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.if (detecting) { const chipMagicValue = await this.readReg(this.CHIP_DETECT_MAGIC_REG_ADDR) >>> 0; this.debug("Chip Magic " + chipMagicValue.toString(16)); const chip = await magic2Chip(chipMagicValue); if (chip === null) { throw new ESPError(`Unexpected CHIP magic value ${chipMagicValue}. Failed to autodetect chip type.`); } else { this.chip = chip; } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/esptool.bundle.js` around lines 7082 - 7092, In the detecting branch of detectChip(), validate the local chip result returned by magic2Chip(chipMagicValue) before assigning it to this.chip. Throw the existing ESPError for a null/unsupported result, and only set this.chip when chip is valid.src/lib/theme-builder.js (1)
2294-2303: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n -A 8 "initThemeBuilder" src/pages/ThemeBuilderPage.jsx rg -n -B2 -A 15 "function pollIconBuild" src/lib/theme-builder.jsRepository: crosspoint-reader/crosspoint-tools
Length of output: 1517
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant implementation around the poll and any teardown helpers. sed -n '1540,1625p' src/lib/theme-builder.js printf '\n----\n' sed -n '2285,2315p' src/lib/theme-builder.js printf '\n----\n' sed -n '1,60p' src/pages/ThemeBuilderPage.jsx # Also search for any destroy/cleanup export usage. printf '\n---- SEARCH ----\n' rg -n "destroyThemeBuilder|cleanup|tbInit|iconPollTimer|renderIconStatus" src/lib/theme-builder.js src/pages/ThemeBuilderPage.jsxRepository: crosspoint-reader/crosspoint-tools
Length of output: 8656
🏁 Script executed:
#!/bin/bash set -euo pipefail # Programmatically inspect whether renderIconStatus touches potentially removed DOM, # and whether any cleanup path exists from the page component. python3 - <<'PY' from pathlib import Path import re theme = Path('src/lib/theme-builder.js').read_text() page = Path('src/pages/ThemeBuilderPage.jsx').read_text() def extract(name): m = re.search(rf'function {name}\(\)\s*\{{', theme) if not m: return None start = m.start() depth = 0 i = m.end() - 1 while i < len(theme): ch = theme[i] if ch == '{': depth += 1 elif ch == '}': depth -= 1 if depth == 0: return theme[start:i+1] i += 1 return None for fn in ['renderIconStatus', 'pollIconBuild', 'initThemeBuilder']: body = extract(fn) print(f'=== {fn} ===') print(body[:2000] if body else 'NOT FOUND') print() print('=== page cleanup mentions ===') for pat in ['return () =>', 'destroyThemeBuilder', 'initThemeBuilder']: print(pat, pat in page) PYRepository: crosspoint-reader/crosspoint-tools
Length of output: 1850
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find the exact call graph around initThemeBuilder and the page effect cleanup. rg -n -A 6 -B 4 "useEffect\(\(\) => \{|initThemeBuilder\(|destroyThemeBuilder|pollIconBuild\(" src/pages/ThemeBuilderPage.jsx src/lib/theme-builder.jsRepository: crosspoint-reader/crosspoint-tools
Length of output: 5446
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1548,1610p' src/lib/theme-builder.js printf '\n----\n' sed -n '2288,2308p' src/lib/theme-builder.js printf '\n----\n' sed -n '1,40p' src/pages/ThemeBuilderPage.jsxRepository: crosspoint-reader/crosspoint-tools
Length of output: 5296
Add cleanup for the icon-build poll timer
initThemeBuilder()only clearsiconPollTimeron re-entry; it doesn’t stop an in-flightpollIconBuild()loop when/theme-builderunmounts. Add a cleanup path so the page stops polling/api/theme-build/statusafter navigation away.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/theme-builder.js` around lines 2294 - 2303, Update initThemeBuilder and the icon-build polling flow around pollIconBuild so unmounting the theme-builder page cancels any in-flight polling and prevents further /api/theme-build/status requests. Return or expose a cleanup function that clears iconPollTimer and invokes it from the page teardown path, while preserving the existing per-canvas initialization guard.src/pages/AdminPage.jsx (2)
980-1013: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
authenticate()only rejects on exactly HTTP 401 — any other failure status is treated as success.If
/api/build/triggerreturns anything other than401or a 2xx (e.g. a 5xx from an upstream hiccup, a WAF/edge challenge, 429), the code skips straight tosetSecret(value); setAuthed(true)without checkingres.ok, granting the dashboard UI even though the secret was never actually confirmed valid. Every subsequent API call in the dashboard would then 401 independently, producing a confusing "logged in but nothing works" state instead of a clear auth error.🐛 Proposed fix
if (res.status === 401) { setAuthError('Invalid secret') setAuthBusy(false) return } + if (!res.ok) { + setAuthError(`Server error (${res.status})`) + setAuthBusy(false) + return + } + const data = await res.json() setSecret(value) setAuthed(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.async function authenticate() { const value = (secretInputRef.current?.value || '').trim() if (!value) return setAuthBusy(true) setAuthError('') // Test the secret by hitting the build trigger endpoint; a 401 means the // secret is wrong. Auth success triggers a build as a side effect (that's ok). try { const res = await fetch('/api/build/trigger', { method: 'POST', headers: { Authorization: `Bearer ${value}` }, }) if (res.status === 401) { setAuthError('Invalid secret') setAuthBusy(false) return } if (!res.ok) { setAuthError(`Server error (${res.status})`) setAuthBusy(false) return } const data = await res.json() setSecret(value) setAuthed(true) log(`Authenticated. Build triggered: ${data.commit || 'unknown'}`) // Save to session. Cards load their own data (status, beta list, // sticky info, banner) when the dashboard mounts. sessionStorage.setItem(STORAGE_KEY, value) } catch { setAuthError('Connection error') setAuthBusy(false) } }🧰 Tools
🪛 ast-grep (0.44.1)
[error] 983-983: React's useState should not be directly called
Context: setAuthBusy(true)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 984-984: React's useState should not be directly called
Context: setAuthError('')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 995-995: React's useState should not be directly called
Context: setAuthError('Invalid secret')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 996-996: React's useState should not be directly called
Context: setAuthBusy(false)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 1001-1001: React's useState should not be directly called
Context: setSecret(value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 1002-1002: React's useState should not be directly called
Context: setAuthed(true)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 1009-1009: React's useState should not be directly called
Context: setAuthError('Connection error')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 1010-1010: React's useState should not be directly called
Context: setAuthBusy(false)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[warning] 1001-1001: Avoid using the initial state variable in setState
Context: setSecret(value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(setstate-same-var)
🪛 React Doctor (0.7.6)
[error] 1008-1008: Storing an auth token in
localStorage/sessionStorageexposes it to any XSS on the page: JavaScript can read web storage and exfiltrate the token. Keep tokens in anHttpOnly,Secure,SameSitecookie instead.Don't persist auth tokens (JWTs, access/refresh tokens, secrets) in
localStorage/sessionStorage; they're readable by any XSS. Use anHttpOnlycookie set by the server.(auth-token-in-web-storage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AdminPage.jsx` around lines 980 - 1013, Update authenticate() to reject every non-2xx response from /api/build/trigger before calling setSecret or setAuthed, while preserving the existing “Invalid secret” message for HTTP 401. For other failed statuses, set an appropriate connection/request error, clear auth busy state, and return; only allow the existing authenticated flow when res.ok is true.
1008-1008: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Admin secret persisted in
sessionStorageis readable by any XSS on the page.Static analysis flags this correctly: any script injected via XSS anywhere on this origin can read
sessionStorageand exfiltrate the admin secret, which grants firmware-build/upload control. This mirrors the legacypublic/admin.htmlpattern (per the comment inadmin/api.js), so it's likely not a regression from this migration, but it's worth tracking as a follow-up to move to anHttpOnly/Secure/SameSitecookie issued by the worker instead of a client-readable bearer token.🧰 Tools
🪛 React Doctor (0.7.6)
[error] 1008-1008: Storing an auth token in
localStorage/sessionStorageexposes it to any XSS on the page: JavaScript can read web storage and exfiltrate the token. Keep tokens in anHttpOnly,Secure,SameSitecookie instead.Don't persist auth tokens (JWTs, access/refresh tokens, secrets) in
localStorage/sessionStorage; they're readable by any XSS. Use anHttpOnlycookie set by the server.(auth-token-in-web-storage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AdminPage.jsx` at line 1008, Remove the client-side persistence of the admin secret in the AdminPage session-storage flow, specifically the sessionStorage.setItem call using STORAGE_KEY. Replace this bearer-token storage with an HttpOnly, Secure, SameSite cookie issued and managed by the worker, updating the associated authentication flow to use that cookie without exposing the secret to page JavaScript.Source: Linters/SAST tools
src/pages/DocsPage.jsx (1)
15-44: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Files ==\n' git ls-files 'src/pages/DocsPage.jsx' 'src/pages/*' | sed -n '1,20p' printf '\n== DocsPage.jsx (relevant slice) ==\n' nl -ba src/pages/DocsPage.jsx | sed -n '1,220p' printf '\n== Search DOMPurify / target / rel usage ==\n' rg -n "DOMPurify|ADD_ATTR|target=|noopener|noreferrer|afterSanitizeAttributes|marked.parse|walkTokens" src -SRepository: crosspoint-reader/crosspoint-tools
Length of output: 792
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== DocsPage.jsx (top 220 lines) ==\n' sed -n '1,220p' src/pages/DocsPage.jsx printf '\n== Search for raw HTML handling and link attributes ==\n' rg -n "DOMPurify|ADD_ATTR|target|rel=|afterSanitizeAttributes|marked\.parse|sanitize\(" src/pages/DocsPage.jsx src -SRepository: crosspoint-reader/crosspoint-tools
Length of output: 25112
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' src/pages/DocsPage.jsx rg -n "DOMPurify|ADD_ATTR|target|rel=|afterSanitizeAttributes|marked\.parse|sanitize\(" src/pages/DocsPage.jsx src -SRepository: crosspoint-reader/crosspoint-tools
Length of output: 25020
Prevent reverse tabnabbing in docs links
ADD_ATTR: ['id', 'target']lets raw docs HTML keeptarget="_blank"without a guaranteedrel="noopener noreferrer". Since this page renders content from the public docs repo, a compromised docs PR can inject a link that can control the opener tab. Add anafterSanitizeAttributeshook to setrelon any<a target="_blank">.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/DocsPage.jsx` around lines 15 - 44, Add an afterSanitizeAttributes hook to the marked configuration in DocsPage.jsx that detects anchor elements with target="_blank" and guarantees rel includes both noopener and noreferrer, while preserving any existing rel values. Apply this to the HTML sanitization flow used for rendered docs content.src/pages/FontsPage.jsx (2)
66-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Unnecessary ref, mutated during render.
familyNameRef.current = familyNameruns unconditionally on every render (flagged by React Doctor's no-ref-current-in-render rule — writes to.currentshould happen in an effect/handler, not render). It's also unneeded here:downloadAllis a plain function recreated each render, so it already closes over the currentfamilyNamestate value directly.🐛 Proposed fix
const pollTimerRef = useRef(null) const builtFilesRef = useRef([]) - const familyNameRef = useRef('') - familyNameRef.current = familyName const logPanelRef = useRef(null)const fam = sanitizeFamilyName(familyNameRef.current) + const fam = sanitizeFamilyName(familyName)Also applies to: 328-336
🧰 Tools
🪛 React Doctor (0.7.6)
[error] 69-69: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/FontsPage.jsx` around lines 66 - 71, Remove the familyNameRef declaration and the unconditional familyNameRef.current assignment from the FontsPage component; update downloadAll and any other references to use the current familyName value from its render closure instead, including the additional usage noted near lines 328-336.Source: Linters/SAST tools
407-647: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
File-input labels missing
htmlFor(broken label/control association).Every text-input label in this file correctly sets
htmlFormatching the input'sid(e.g.familyName,customIntervals,size${i}), but the file-input labels (fontFolder,fontRegular,fontBold,fontItalic,fontBoldItalic,fallbackFontFolder,fallbackFontRegular,fallback2FontFolder,fallback2FontRegular) omit it. Screen readers won't announce these labels when the corresponding input receives focus, and clicking the label text won't activate the file picker.🛠️ Example fix (repeat for each file-input label)
- <label className="block text-sm/6 font-medium text-stone-700"> + <label htmlFor="fontFolder" className="block text-sm/6 font-medium text-stone-700"> Quick add from folder </label>- <label className="block text-sm/6 font-medium text-stone-700"> + <label htmlFor="fontRegular" className="block text-sm/6 font-medium text-stone-700"> Regular <span className="text-red-500">*</span> </label>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/FontsPage.jsx` around lines 407 - 647, Add htmlFor attributes to every file-input label in the FontsPage JSX, matching each label to its corresponding input id: fontFolder, fontRegular, fontBold, fontItalic, fontBoldItalic, fallbackFontFolder, fallbackFontRegular, fallback2FontFolder, and fallback2FontRegular. Preserve the existing labels and input behavior.src/pages/home/FlashTools.jsx (1)
286-334: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restart instructions shown before flashing succeeds.
setRestart(...)fires unconditionally before the firmware is even downloaded or flashed. Any failure in the try block (missing file, fetch error, flasher error) still leaves the "After flashing" unplug/reset card visible, misleading the user into power-cycling a device that was never flashed.🐛 Proposed fix
setRunning(true) - setRestart({ unplug: skipReset && model !== 'x4' }) setPercent(0) setProgress({ title, steps, states: [...states], status: downloadMsg ? { kind: 'info', text: downloadMsg } : null }) try { ... setProgress((p) => ({ ...p, status: { kind: 'ok', text: skipReset ? 'Flash complete! Unplug and replug the USB cable to restart your device.' : 'Flash complete! Your device will restart with the new firmware.', }, })) + setRestart({ unplug: skipReset && model !== 'x4' }) } catch (err) { setProgress((p) => ({ ...p, status: { kind: 'err', text: err.message } })) }🧰 Tools
🪛 ast-grep (0.44.1)
[error] 286-286: React's useState should not be directly called
Context: setRestart({ unplug: skipReset && model !== 'x4' })
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 287-287: React's useState should not be directly called
Context: setPercent(0)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 288-288: React's useState should not be directly called
Context: setProgress({ title, steps, states: [...states], status: downloadMsg ? { kind: 'info', text: downloadMsg } : null })
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 307-307: React's useState should not be directly called
Context: setProgress((p) => ({ ...p, status: null }))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 321-329: React's useState should not be directly called
Context: setProgress((p) => ({
...p,
status: {
kind: 'ok',
text: skipReset
? 'Flash complete! Unplug and replug the USB cable to restart your device.'
: 'Flash complete! Your device will restart with the new firmware.',
},
}))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 331-331: React's useState should not be directly called
Context: setProgress((p) => ({ ...p, status: { kind: 'err', text: err.message } }))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
[error] 334-334: React's useState should not be directly called
Context: setRunning(false)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/home/FlashTools.jsx` around lines 286 - 334, Move the setRestart call from the initial setup into the successful completion path after flasher.flashFirmware resolves. Keep it unset or false while firmware fetching/flashing is in progress and when the try block fails, and preserve the existing unplug condition based on skipReset and model !== 'x4'.src/pages/home/markdown.js (1)
14-18: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
safeUrlaccepts protocol-relative URLs (//evil.com).The
\/alternative in the allowlist regex matches any string starting with a single/, including//hostprotocol-relative URLs, which browsers resolve as absolute links to an external origin. This defeats the intent of allowlisting only same-origin root-relative paths. This helper's output feedshrefattributes rendered viadangerouslySetInnerHTML(e.g. beta build notes insrc/pages/home/FlashTools.jsx), so closing this gap matters for the whole call chain.🛡️ Proposed fix
function safeUrl(url) { const trimmed = url.trim() - if (/^(https?:\/\/|\/|#|mailto:)/i.test(trimmed)) return trimmed + if (/^(https?:\/\/|\/(?!\/)|#|mailto:)/i.test(trimmed)) return trimmed return '#' }📝 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.function safeUrl(url) { const trimmed = url.trim() if (/^(https?:\/\/|\/(?!\/)|#|mailto:)/i.test(trimmed)) return trimmed return '#' }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/home/markdown.js` around lines 14 - 18, Update safeUrl so the root-relative URL branch accepts a single leading slash but rejects protocol-relative values beginning with “//”. Preserve the existing handling for http(s), hash, and mailto URLs, and continue returning “#” for disallowed inputs.src/pages/insider/buildShared.jsx (1)
30-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against missing
messageto avoid a hard crash.
message.split('\n')throws ifmessageisundefined/null. Any malformed changelog entry from the build-meta API would crash the wholeChangelogListrender.🛡️ Proposed guard
export function cleanCommitMessage(message) { - const msg = message + const msg = (message || '') .split('\n')[0] .replace(/^(feat|fix|chore|refactor|docs|style|test|perf|ci|build|revert)(\(.+?\))?:\s*/i, '') return msg.charAt(0).toUpperCase() + msg.slice(1) }📝 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 function cleanCommitMessage(message) { const msg = (message || '') .split('\n')[0] .replace(/^(feat|fix|chore|refactor|docs|style|test|perf|ci|build|revert)(\(.+?\))?:\s*/i, '') return msg.charAt(0).toUpperCase() + msg.slice(1) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/insider/buildShared.jsx` around lines 30 - 35, Update cleanCommitMessage to handle nullish or missing message values before calling split, returning a safe empty or established fallback string while preserving the current formatting behavior for valid messages.src/pages/InsiderPage.jsx (3)
88-94: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wrap
loadBuildInfoin try/catch to avoid a stuck loading spinner.Unlike the sibling effect at lines 100-115, this has no error handling. If
fetchBuildMeta()rejects,setLoading(false)is skipped and the page is stuck showing the loading spinner indefinitely — the only retry UI (the "Refresh" button) is gated behind!loading, so there's no way to recover without a full page reload.🛡️ Proposed fix
const loadBuildInfo = useCallback(async () => { setLoading(true) setMeta(null) - const data = await fetchBuildMeta() - setLoading(false) - setMeta(data || null) + try { + const data = await fetchBuildMeta() + setMeta(data || null) + } catch { + setMeta(null) + } finally { + setLoading(false) + } }, [])📝 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 loadBuildInfo = useCallback(async () => { setLoading(true) setMeta(null) try { const data = await fetchBuildMeta() setMeta(data || null) } catch { setMeta(null) } finally { setLoading(false) } }, [])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/InsiderPage.jsx` around lines 88 - 94, Update loadBuildInfo in InsiderPage to wrap fetchBuildMeta and the success state updates in try/catch/finally, ensuring setLoading(false) always executes when the request rejects so the refresh UI remains available. Preserve the existing metadata reset and successful setMeta behavior, and follow the sibling effect’s established error-handling approach where applicable.
205-231: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
TOCTOU race:
cfPollingRef.currentis checked before the await, not re-checked after.Both
poll()(lines 205-231) and thevisibilitychangehandler (lines 281-300) guard oncfPollingRef.currentbefore callingfetchCustomBuildStatus(), but not after the await resolves. IfclearCustomBuild()sets the ref tofalsewhile a request from either path is in flight, the stale response still runsshowCustomBuildReady()/setCfError/setCfState('failed'), silently reviving state the user just cleared.🛡️ Proposed fix (apply to both call sites)
async function poll() { if (!cfPollingRef.current) return try { const build = await fetchCustomBuildStatus() + if (!cfPollingRef.current) return // re-check after the await if (build) {fetchCustomBuildStatus() .then((build) => { - if (!build) return + if (!cfPollingRef.current || !build) return // re-check after the await if (build.status === 'success') {Also applies to: 281-300
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 227-227: Avoid using the initial state variable in setState
Context: setTimeout(poll, 10000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/InsiderPage.jsx` around lines 205 - 231, Re-check cfPollingRef.current immediately after each fetchCustomBuildStatus() await in both poll() and the visibilitychange handler, returning without applying results when polling was cleared during the request. Ensure stale responses cannot call showCustomBuildReady, setCfError, or setCfState after clearCustomBuild().
733-752: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard
parseIntagainstNaNin font-size inputs.
parseInt(e.target.value)with no fallback lets a momentarily-empty field pushNaNintocfSizes. InhandleCfBuild,vals.some((v, i) => v !== defaults[i])(line 337) is alwaystruefor aNaNentry, so an invalid size can be uploaded to the build backend.🛡️ Proposed fix
onChange={(e) => setCfSizes((prev) => { const next = [...(prev[family] || sizes)] - next[i] = parseInt(e.target.value) + const parsed = parseInt(e.target.value, 10) + next[i] = Number.isNaN(parsed) ? sizes[i] : parsed return { ...prev, [family]: next } }) }📝 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.{sizes.map((sz, i) => ( <div key={i} className="flex-1"> <label className="text-[10px] text-stone-400"> {SIZE_LABELS[i]} </label> <input type="number" min="6" max="30" value={sz} onChange={(e) => setCfSizes((prev) => { const next = [...(prev[family] || sizes)] const parsed = parseInt(e.target.value, 10) next[i] = Number.isNaN(parsed) ? sizes[i] : parsed return { ...prev, [family]: next } }) } className="w-full rounded border border-stone-200 bg-white px-1.5 py-1 text-center text-xs text-stone-700 tabular-nums focus:border-brand-300 focus:ring-1 focus:ring-brand-500/20 focus:outline-none" /> </div> ))🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 734-736: A list component should have a key to prevent re-rendering
Context:
{SIZE_LABELS[i]}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(list-component-needs-key)
[warning] 737-750: A list component should have a key to prevent re-rendering
Context: <input
type="number"
min="6"
max="30"
value={sz}
onChange={(e) =>
setCfSizes((prev) => {
const next = [...(prev[family] || sizes)]
next[i] = parseInt(e.target.value)
return { ...prev, [family]: next }
})
}
className="w-full rounded border border-stone-200 bg-white px-1.5 py-1 text-center text-xs text-stone-700 tabular-nums focus:border-brand-300 focus:ring-1 focus:ring-brand-500/20 focus:outline-none"
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(list-component-needs-key)
[warning] 732-752: Do not use array indexes for a list component's key
Context: sizes.map((sz, i) => (
{SIZE_LABELS[i]}
<input
type="number"
min="6"
max="30"
value={sz}
onChange={(e) =>
setCfSizes((prev) => {
const next = [...(prev[family] || sizes)]
next[i] = parseInt(e.target.value)
return { ...prev, [family]: next }
})
}
className="w-full rounded border border-stone-200 bg-white px-1.5 py-1 text-center text-xs text-stone-700 tabular-nums focus:border-brand-300 focus:ring-1 focus:ring-brand-500/20 focus:outline-none"
/>
))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(list-component-no-index)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/InsiderPage.jsx` around lines 733 - 752, Update the size input handler in the sizes.map callback to prevent empty or invalid values from storing NaN in cfSizes. Parse the entered value and retain the previous valid size, or apply an appropriate existing fallback, when parsing fails; preserve valid numeric updates so handleCfBuild receives only usable font sizes.src/pages/LoginPage.jsx (1)
41-47: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Move the
setTimeoutside effect out of the state updater.
setShowLogin's functional updater performs a side effect (setTimeout); React may invoke updaters more than once, so this side effect isn't guaranteed to run exactly once. Since the currentshowLoginvalue is already in scope, a functional updater isn't needed here.🔧 Proposed fix
function toggleLogin() { - setShowLogin((prev) => { - const next = !prev - if (next) setTimeout(() => emailRef.current?.focus(), 0) - return next - }) + const next = !showLogin + setShowLogin(next) + if (next) setTimeout(() => emailRef.current?.focus(), 0) }📝 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.function toggleLogin() { const next = !showLogin setShowLogin(next) if (next) setTimeout(() => emailRef.current?.focus(), 0) }🧰 Tools
🪛 ast-grep (0.44.1)
[error] 41-45: React's useState should not be directly called
Context: setShowLogin((prev) => {
const next = !prev
if (next) setTimeout(() => emailRef.current?.focus(), 0)
return next
})
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.(usestate-direct-usage)
🪛 React Doctor (0.7.6)
[error] 42-42: This state updater performs setTimeout(). React may run updater functions more than once, so side effects here can repeat or observe inconsistent external state.
Keep state updater callbacks pure and return only the next state. Move notifications, storage, timers, ref writes, and other external work into the event or effect that queues the update.
(no-impure-state-updater)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/LoginPage.jsx` around lines 41 - 47, Update toggleLogin so it derives the next visibility value from the current showLogin state, schedules the emailRef focus outside the setShowLogin updater when opening the login view, and then applies the next value without side effects inside the updater.Source: Linters/SAST tools
src/pages/StickyPage.jsx (1)
16-51: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Escape quotes before interpolating URLs into HTML
escapeHtml()only handles text-node escaping, butrenderMarkdown()later drops the URL directly intohref="...". A markdown link containing"can break out of the attribute and inject arbitrary attributes;safeUrl()also lets protocol-relative URLs like//evil.comthrough. Block//URLs and consider reusing themarked+DOMPurifypipeline already used inDocsPage.jsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/StickyPage.jsx` around lines 16 - 51, Update safeUrl() to reject protocol-relative URLs beginning with //, and add attribute-safe escaping for URLs before interpolating them into href values in renderMarkdown(). Apply this to both markdown links and bare URL links, preserving the existing allowed schemes and rendering behavior; reuse the established marked/DOMPurify pipeline only if it is already available for this page.worker/types.ts (1)
9-9: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n "ALLOW_INSECURE_DEV_WEBHOOKS" worker wrangler.jsoncRepository: crosspoint-reader/crosspoint-tools
Length of output: 482
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant implementation and deployment config. sed -n '280,340p' worker/index.ts printf '\n---\n' sed -n '1,220p' wrangler.jsonc printf '\n---\n' rg -n "production|dev|preview|ALLOW_INSECURE_DEV_WEBHOOKS|webhook" worker/index.ts wrangler.jsoncRepository: crosspoint-reader/crosspoint-tools
Length of output: 5693
🏁 Script executed:
#!/bin/bash set -euo pipefail # Look for any runtime environment gating around the webhook bypass flag. ast-grep outline worker/index.ts --view expanded printf '\n---\n' rg -n "env\.(NODE_ENV|ENVIRONMENT|WRANGLER|preview|production|dev)|process\.env|import\.meta\.env|ALLOW_INSECURE_DEV_WEBHOOKS" worker/index.ts worker/types.ts wrangler.jsoncRepository: crosspoint-reader/crosspoint-tools
Length of output: 6737
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the relevant section plus any deployment/env config that might restrict the flag. sed -n '300,325p' worker/index.ts printf '\n---\n' sed -n '1,220p' wrangler.jsonc printf '\n---\n' rg -n "ALLOW_INSECURE_DEV_WEBHOOKS|environment|vars|preview|production|dev" -g 'wrangler*' -g '*.json' -g '*.jsonc' -g '*.toml' .Repository: crosspoint-reader/crosspoint-tools
Length of output: 33608
Restrict
ALLOW_INSECURE_DEV_WEBHOOKSto dev-only.worker/index.tsreads this env var directly inisAuthorizedWebhookRequest, so a production secret can disable webhook signature checks. Gate it behind a dev/preview-only check or remove it from production bindings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@worker/types.ts` at line 9, Restrict ALLOW_INSECURE_DEV_WEBHOOKS to development or preview environments in isAuthorizedWebhookRequest, ensuring production cannot disable webhook signature validation. Alternatively, remove this variable from production bindings while preserving its intended dev-only behavior.worker/webhook.ts (1)
3-25: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use constant-time HMAC verification here instead of
===.
signature === expectedcompares the hex strings with short-circuiting string equality, which leaks timing on a security-sensitive check. Switch tocrypto.subtle.verify()to keep the comparison constant-time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@worker/webhook.ts` around lines 3 - 25, Update verifyGitHubSignature to use crypto.subtle.verify() with the imported HMAC key and decoded x-hub-signature-256 value instead of constructing expected and comparing with ===. Preserve the existing sha256= prefix validation and return the request body alongside the verification result.
fix: add local env
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/index.css (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse string notation for
@import.To align with standard Stylelint conventions (
import-notation), prefer using string notation directly instead of theurl()function for@import.♻️ Proposed fix
-@import url('https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap'); +@import 'https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap'; `@import` "tailwindcss";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.css` around lines 1 - 2, Update the font `@import` in the stylesheet to use direct quoted-string notation instead of wrapping the URL in `url()`. Keep the imported Google Fonts URL and the existing Tailwind import unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/index.css`:
- Around line 1-2: Update the font `@import` in the stylesheet to use direct
quoted-string notation instead of wrapping the URL in `url()`. Keep the imported
Google Fonts URL and the existing Tailwind import unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9cc56132-e84a-448b-a391-5f6c9fce3bc9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonsrc/components/Header.jsxsrc/index.csssrc/pages/docs/docs-prose.csssrc/pages/home/Hero.jsxvite.config.js
🚧 Files skipped from review as they are similar to previous changes (4)
- vite.config.js
- src/pages/docs/docs-prose.css
- package.json
- src/pages/home/Hero.jsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Workers Builds: crosspoint-tools
⚠️ CI failures not shown inline (1)
GitHub Check: Workers Builds: crosspoint-tools: Workers Builds: crosspoint-tools
Conclusion: failure
Build ID: [90fb49f2-c319-4811-8227-97bbb8c6719f](https://dash.cloudflare.com/73f82799694e2fad048f544e0be28c1c/workers/services/view/crosspoint-tools/production/builds/90fb49f2-c319-4811-8227-97bbb8c6719f)
Script: [crosspoint-tools](https://dash.cloudflare.com/73f82799694e2fad048f544e0be28c1c/workers/services/view/crosspoint-tools/production)
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/components/Header.jsx
[warning] 65-65: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 69-69: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🪛 Stylelint (17.14.0)
src/index.css
[error] 1-1: Expected "url('https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap')" to be "'https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap'" (import-notation)
(import-notation)
[error] 4-4: Unexpected unknown at-rule "@theme" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🔇 Additional comments (5)
src/components/Header.jsx (3)
4-44: LGTM!
46-93: LGTM!
95-117: LGTM!src/index.css (2)
4-11: LGTM!
110-120: LGTM!
Replace Tailwind-CDN static HTML pages with a React SPA (React Router 7) built by Vite with @cloudflare/vite-plugin; worker moved to worker/
All 12 pages ported as routes with copy, images, and behavior preserved (WebSerial flasher, font builder, theme builder, debug console, admin, insider/login, kosync, sticky, unlocker, docs, roadmap)
Shared design system: brand-green tokens on Free-Ink patterns shared Header/Footer/Layout/Modal components - Legacy .html URLs 301-redirect to SPA routes; SPA fallback enabled