feat: add serverlist - #243
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…preset to serverlist
…pagination to a standalone component
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
birdflop-com | 449a129 | Aug 01 2026, 08:12 PM |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Minecraft server list with persistent storage, submission and editing, filtering, live Java and Bedrock status checks, voting, reporting, verification, BBCode and MOTD rendering, plugin synchronization, and a publishable status package. ChangesServer List Platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant ServerList
participant QueryServers
participant ServerStatus
participant Database
Visitor->>ServerList: request filtered server list
ServerList->>QueryServers: pass filters and pagination
QueryServers->>Database: query servers and vote totals
QueryServers->>ServerStatus: fetch live statuses when required
ServerStatus-->>QueryServers: return normalized statuses
QueryServers-->>ServerList: return ranked server results
ServerList-->>Visitor: render server cards
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/resources/plugins/index.tsx (1)
177-233: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftNo debounce on writes triggered for every store mutation.
deepTrack(track, pluginsStore)reruns this task on any change topluginsStore, including UI-only changes like switchingopenServerorfilter. For authenticated users, each rerun now also callssetUserDataand, when a server is linked,updateServerPlugins. Previously this task only wrote tolocalStorage, which is cheap; it now issues network requests to persist the whole store on every interaction, with no debouncing and no guard against overlapping in-flight writes if the store changes again before a previous write resolves.Debounce the network persistence, or split UI-only state (
openServer,filter) from plugin-content state so trivial interactions do not trigger asetUserData/updateServerPluginsround trip.🤖 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/routes/resources/plugins/index.tsx` around lines 177 - 233, Debounce the authenticated network persistence in the pluginsStore task so UI-only mutations such as openServer or filter do not trigger immediate requests. Keep localStorage persistence responsive, but schedule setUserData and updateServerPlugins through a shared debounce that coalesces changes and prevents overlapping in-flight writes, using the existing pluginsStore task and persistence symbols.
🧹 Nitpick comments (2)
src/components/ServerList/ServerCard.tsx (1)
53-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the address formatting into a helper.
The nested ternary computes the Java address and the Bedrock address in one expression.
src/routes/serverlist/[slug]/index.tsxlines 116-121 repeats the same rule withDEFAULT_JAVA_PORTandDEFAULT_BEDROCK_PORT. Move the logic to a shared function insrc/util/serverlist/and use the constants here too, so the two views cannot diverge.♻️ Suggested shape
// src/util/serverlist/address.ts export function formatAddress( host: string, port: number | null, defaultPort: number ): string { return port && port !== defaultPort ? `${host}:${port}` : host; }🤖 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/ServerList/ServerCard.tsx` around lines 53 - 65, Extract the nested address-formatting logic from the ServerCard displayIp calculation into a shared formatAddress helper under src/util/serverlist/, and reuse it for both Java and Bedrock addresses. Update ServerCard and the server-list route to use DEFAULT_JAVA_PORT and DEFAULT_BEDROCK_PORT through this helper, preserving omission of default ports and formatting only the selected primary host.src/routes/serverlist/[slug]/index.tsx (1)
90-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
Outputfor the copy control, and handle the clipboard rejection.
navigator.clipboard?.writeText(address)returns an unhandled promise. A denied clipboard permission fails silently, and the user gets no feedback.src/components/Elements/Output.tsxalready implements copy with notifications. Use that component here instead of the local button.🤖 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/routes/serverlist/`[slug]/index.tsx around lines 90 - 111, Replace the local copy button in ConnectRow with the existing Output component, passing address as its displayed/copyable value and preserving the current row styling and label. Remove the direct navigator.clipboard call so clipboard failures are handled by Output’s notification 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 `@drizzle/schema.ts`:
- Around line 159-223: Prevent votifierToken from reaching clients by replacing
whole-row selections of servers in the server-list query flow and the server
detail route with explicit public-column projections. Exclude votifierToken from
those projections while preserving all fields required by the listing UI; retain
access only in privileged backend paths that need to deliver votes.
In `@src/components/Elements/Output.tsx`:
- Around line 38-52: Update the compact branch in the Output component to use a
keyboard-accessible button with button semantics while preserving the existing
click behavior, styling, value, and copy icon. Keep the control labeled with the
translated “Copy IP” title by routing that string through inlineTranslate,
consistent with the other localized strings in the component.
- Around line 23-36: Update the onClick handler in Output.tsx so
notifications.push(notification.toJSON()) runs after
navigator.clipboard.writeText(value) resolves or rejects, ensuring the
catch-updated failure notification snapshot is pushed. Preserve the green
success notification for successful copies and the existing error details and
red styling for failures.
In `@src/components/ServerList/StatusBadge.tsx`:
- Around line 13-39: Update the players display condition in the StatusBadge
component so it narrows status before accessing player data, and ensure the
status.players.online and status.players.max reads remain safe when status or
players is null. Preserve the existing rendering behavior for online servers
with showPlayers enabled.
In `@src/components/ServerList/Turnstile.tsx`:
- Around line 57-76: Update the Turnstile effect around render and cleanup so
the script element and load-handler state are accessible to ctx.cleanup. Remove
the load listener during cleanup and guard render with an active-mounted flag,
ensuring the script cannot create a widget after unmount while preserving normal
immediate and script-load rendering.
In `@src/routes/resources/plugins/index.tsx`:
- Around line 134-152: Update the database plugin-state condition in the
useVisibleTask$ initialization flow to distinguish missing data from an
intentionally empty sync. Treat any defined dbPlugins value, including one with
an empty servers object, as authoritative and restore its stored fields; only
enter the localStorage migration/setUserData fallback when dbPlugins itself is
undefined.
In `@src/routes/serverlist/`[slug]/index.tsx:
- Around line 72-76: Annotate the session value retrieved in the
server-management checks as Session | null, using the existing Session type from
src/util/dataUtils.ts. Apply this in src/routes/serverlist/[slug]/index.tsx
lines 72-76 and src/routes/serverlist/[slug]/edit/index.tsx lines 21-24 before
accessing session.user, without changing the canManage logic.
In `@src/routes/serverlist/index.tsx`:
- Around line 76-86: Update the error handling in the server-loading function to
log the caught error server-side, but replace error: String(err) with a fixed
user-safe message before the result reaches the page rendering at line 356. Keep
the existing empty rows, total, statuses, liveRefined, and params behavior
unchanged.
- Around line 31-35: Validate the numeric results used to derive page and
perPage in the server-list pagination setup. Update the page and perPage parsing
expressions so non-numeric query values fall back to their existing defaults
before applying the Math.max/Math.min bounds, ensuring valid values reach limit,
offset, and totalPages.
In `@src/util/serverlist/actions.ts`:
- Around line 271-301: Update reportServer to validate a Turnstile token using
the existing verifyTurnstile pattern from voteForServer, and enforce the same
24-hour username/IP cooldown before inserting into serverReports. Preserve the
existing validation and response behavior, and reject submissions that fail
CAPTCHA or rate-limit checks before the database insert.
- Around line 352-370: Update setServerVerified to look up the server by
serverId before performing the update, following the existing
updateServerPlugins pattern. Return the established not-found failure response
when no matching server exists, and only return success after updating an
existing row.
- Around line 47-370: Add a shared session accessor that casts
sharedMap.get('session') to Session | null, then replace the direct session
lookups in createServer, updateServer, deleteServer, reportServer, and
updateServerPlugins with this helper. Import Session from `@auth/qwik` and
preserve the existing null-safe authorization behavior.
- Line 88: Standardize all isAdmin invocations in the caller actions across the
serverlist and data-utils modules. Use the bound server$ call form consistently
so each admin check receives the request context and can access this.sharedMap,
including the invocation near the existing const admin assignment.
In `@src/util/serverlist/bbcode.ts`:
- Around line 73-105: Type every regex replacement callback parameter currently
inferred as any in the BBCode conversion chain, including color/size, URL
href/label, image src, and list callback parameters, as string. Update the
callbacks around the existing safeColor, safeSize, safeUrl, and related list
handling without changing their behavior.
In `@src/util/serverlist/queries.ts`:
- Around line 96-120: Prevent sensitive server and owner fields from reaching
public Qwik loader payloads: in src/util/serverlist/queries.ts lines 96-120 and
src/routes/serverlist/[slug]/index.tsx lines 36-88, replace full-row selections
with explicit public column projections excluding servers.votifierToken and
owner PII; in src/routes/serverlist/[slug]/edit/index.tsx lines 12-27, only
return listing data when canManage is true and exclude votifierToken; in
drizzle/schema.ts lines 159-223, either move Votifier credentials into a
separate table or document the invariant that votifierToken is never selected by
public read paths.
In `@src/util/serverlist/status.ts`:
- Around line 57-67: Update the fetch call in getServerStatus to pass an
AbortSignal.timeout using the appropriate request duration, and ensure an
aborted request follows the existing fallback path. Preserve the current
response validation and McStatusResponse handling for requests that complete
normally.
In `@src/util/serverlist/turnstile.ts`:
- Around line 27-44: Update the Turnstile verification fetch in verifyTurnstile
to use an AbortController with a 5-second timeout, passing its signal to fetch
and aborting when the timer expires. Clear the timeout after the fetch completes
or fails, while preserving the existing success, failure, and catch handling.
In `@src/util/serverlist/validation.ts`:
- Around line 75-82: Update parsePort to validate string inputs after trimming,
requiring the complete value to consist only of decimal digits before
conversion. Replace the partial parseInt behavior with Number() after this
check, while preserving existing undefined, empty, null, integer, and 1–65535
range handling.
- Around line 94-95: Update server destination validation around HOST_RE to
resolve hostnames and reject private or reserved IP ranges, including localhost,
loopback, RFC 1918, link-local, unspecified, and equivalent IPv6 addresses,
before any status check or Votifier message is sent. Ensure status.ts validates
the resolved destination before fetch() and sendVotifierV2() performs the same
protection before creating the cloudflare:sockets TCP connection.
In `@src/util/serverlist/votifier.ts`:
- Around line 75-91: Update the socket declaration and the dynamically imported
connect return type in the votifier flow to use ReadableStream<Uint8Array> and
WritableStream<Uint8Array> instead of unparameterized stream types, ensuring
reader.read().value is typed for TextDecoder.decode without call-site casts.
---
Outside diff comments:
In `@src/routes/resources/plugins/index.tsx`:
- Around line 177-233: Debounce the authenticated network persistence in the
pluginsStore task so UI-only mutations such as openServer or filter do not
trigger immediate requests. Keep localStorage persistence responsive, but
schedule setUserData and updateServerPlugins through a shared debounce that
coalesces changes and prevents overlapping in-flight writes, using the existing
pluginsStore task and persistence symbols.
---
Nitpick comments:
In `@src/components/ServerList/ServerCard.tsx`:
- Around line 53-65: Extract the nested address-formatting logic from the
ServerCard displayIp calculation into a shared formatAddress helper under
src/util/serverlist/, and reuse it for both Java and Bedrock addresses. Update
ServerCard and the server-list route to use DEFAULT_JAVA_PORT and
DEFAULT_BEDROCK_PORT through this helper, preserving omission of default ports
and formatting only the selected primary host.
In `@src/routes/serverlist/`[slug]/index.tsx:
- Around line 90-111: Replace the local copy button in ConnectRow with the
existing Output component, passing address as its displayed/copyable value and
preserving the current row styling and label. Remove the direct
navigator.clipboard call so clipboard failures are handled by Output’s
notification 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b7489f4-3bea-4f91-8879-3d5c06bafeb4
📒 Files selected for processing (43)
drizzle/migrations/0006_reflective_tarantula.sqldrizzle/migrations/0007_burly_madame_web.sqldrizzle/migrations/0008_safe_nightshade.sqldrizzle/migrations/0009_soft_krista_starr.sqldrizzle/migrations/0010_stale_sumo.sqldrizzle/migrations/0011_wet_stranger.sqldrizzle/migrations/meta/0006_snapshot.jsondrizzle/migrations/meta/0007_snapshot.jsondrizzle/migrations/meta/0008_snapshot.jsondrizzle/migrations/meta/0009_snapshot.jsondrizzle/migrations/meta/0010_snapshot.jsondrizzle/migrations/meta/0011_snapshot.jsondrizzle/migrations/meta/_journal.jsondrizzle/schema.tssrc/components/Elements/Nav.tsxsrc/components/Elements/Output.tsxsrc/components/ServerList/BBCodeEditor.tsxsrc/components/ServerList/ServerCard.tsxsrc/components/ServerList/ServerControls.tsxsrc/components/ServerList/ServerForm.tsxsrc/components/ServerList/StatusBadge.tsxsrc/components/ServerList/Turnstile.tsxsrc/components/ServerList/VoteSection.tsxsrc/components/rgbirdflop/presets/MyPrivatePresets.tsxsrc/routes/plugin@auth.tssrc/routes/resources/plugins/index.tsxsrc/routes/resources/rgb/index.tsxsrc/routes/serverlist/[slug]/edit/index.tsxsrc/routes/serverlist/[slug]/index.tsxsrc/routes/serverlist/index.tsxsrc/routes/serverlist/submit/index.tsxsrc/types.d.tssrc/util/dataUtils.tssrc/util/plugins/types.tssrc/util/serverlist/__tests__/validation.test.tssrc/util/serverlist/actions.tssrc/util/serverlist/bbcode.tssrc/util/serverlist/constants.tssrc/util/serverlist/queries.tssrc/util/serverlist/status.tssrc/util/serverlist/turnstile.tssrc/util/serverlist/validation.tssrc/util/serverlist/votifier.ts
| const onClick = () => { | ||
| const notification = new Notification() | ||
| .setTitle(copiedTitle) | ||
| .setDescription(copiedDescription) | ||
| .setBgColor('lum-grad-bg-green/50'); | ||
| navigator.clipboard.writeText(value).catch((err: unknown) => { | ||
| notification | ||
| .setTitle(copyFailedTitle) | ||
| .setDescription(err instanceof Error ? err.message : String(err)) | ||
| .setBgColor('lum-grad-bg-red/50') | ||
| .setPersist(true); | ||
| }); | ||
| notifications.push(notification.toJSON()); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The copy-failure notification never reaches the user.
notifications.push(notification.toJSON()) runs synchronously and pushes a plain snapshot object. The catch callback runs later and mutates the notification instance, not the pushed snapshot. On a clipboard failure the user still sees the green "copied" notification. Push the notification inside the promise resolution instead.
🐛 Proposed fix
const onClick = () => {
const notification = new Notification()
.setTitle(copiedTitle)
.setDescription(copiedDescription)
.setBgColor('lum-grad-bg-green/50');
- navigator.clipboard.writeText(value).catch((err: unknown) => {
- notification
- .setTitle(copyFailedTitle)
- .setDescription(err instanceof Error ? err.message : String(err))
- .setBgColor('lum-grad-bg-red/50')
- .setPersist(true);
- });
- notifications.push(notification.toJSON());
+ navigator.clipboard
+ .writeText(value)
+ .then(() => notifications.push(notification.toJSON()))
+ .catch((err: unknown) => {
+ notifications.push(
+ notification
+ .setTitle(copyFailedTitle)
+ .setDescription(err instanceof Error ? err.message : String(err))
+ .setBgColor('lum-grad-bg-red/50')
+ .setPersist(true)
+ .toJSON()
+ );
+ });
};📝 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 onClick = () => { | |
| const notification = new Notification() | |
| .setTitle(copiedTitle) | |
| .setDescription(copiedDescription) | |
| .setBgColor('lum-grad-bg-green/50'); | |
| navigator.clipboard.writeText(value).catch((err: unknown) => { | |
| notification | |
| .setTitle(copyFailedTitle) | |
| .setDescription(err instanceof Error ? err.message : String(err)) | |
| .setBgColor('lum-grad-bg-red/50') | |
| .setPersist(true); | |
| }); | |
| notifications.push(notification.toJSON()); | |
| }; | |
| const onClick = () => { | |
| const notification = new Notification() | |
| .setTitle(copiedTitle) | |
| .setDescription(copiedDescription) | |
| .setBgColor('lum-grad-bg-green/50'); | |
| navigator.clipboard | |
| .writeText(value) | |
| .then(() => notifications.push(notification.toJSON())) | |
| .catch((err: unknown) => { | |
| notifications.push( | |
| notification | |
| .setTitle(copyFailedTitle) | |
| .setDescription(err instanceof Error ? err.message : String(err)) | |
| .setBgColor('lum-grad-bg-red/50') | |
| .setPersist(true) | |
| .toJSON() | |
| ); | |
| }); | |
| }; |
🤖 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/Elements/Output.tsx` around lines 23 - 36, Update the onClick
handler in Output.tsx so notifications.push(notification.toJSON()) runs after
navigator.clipboard.writeText(value) resolves or rejects, ensuring the
catch-updated failure notification snapshot is pushed. Preserve the green
success notification for successful copies and the existing error details and
red styling for failures.
| const online = !!status?.online; | ||
| return ( | ||
| <div class="flex items-center gap-2 text-sm"> | ||
| <span | ||
| class={{ | ||
| 'flex items-center gap-1.5 font-semibold': true, | ||
| 'text-green-400': online, | ||
| 'text-lum-text-secondary': !online, | ||
| }} | ||
| > | ||
| <Circle | ||
| size={10} | ||
| class={ | ||
| online | ||
| ? 'fill-green-400 text-green-400' | ||
| : 'fill-lum-text-secondary text-lum-text-secondary' | ||
| } | ||
| /> | ||
| {online ? 'Online' : 'Offline'} | ||
| </span> | ||
| {online && showPlayers && ( | ||
| <span class="text-lum-text-secondary flex items-center gap-1.5"> | ||
| <Users size={14} /> | ||
| {status.players.online.toLocaleString()} | ||
| <span class="opacity-50"> | ||
| / {status.players.max.toLocaleString()} | ||
| </span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -H -t f 'tsconfig*.json' --exec sh -c 'echo "== $1"; cat "$1"' _ {}Repository: birdflop/web
Length of output: 2068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -H 'StatusBadge.tsx' .
echo "== file outline =="
ast-grep outline src/components/ServerList/StatusBadge.tsx --view expanded || true
echo "== relevant file contents =="
cat -n src/components/ServerList/StatusBadge.tsx
echo "== search ServerStatus definition =="
rg -n "ServerStatus|type .*Status|interface .*Status" src packages -g '*.ts' -g '*.tsx' || trueRepository: birdflop/web
Length of output: 3579
🌐 Web query:
TypeScript strictNullChecks boolean variable narrow null inside conditional non-null assertion
💡 Result:
When strictNullChecks is enabled, TypeScript uses control flow-based type analysis to narrow variables [1]. When you perform a conditional check on a variable that might be null or undefined (e.g., if (variable)), TypeScript narrows the type within that block based on truthiness [2][3][4]. To safely handle a variable that could be null or undefined, you should prefer explicit type guards over non-null assertions whenever possible [5][6]. 1. Truthiness Narrowing: Using if (variable) will narrow the type by removing null and undefined, as they are falsy values [2][7][4]. However, be cautious if your variable could be an empty string or the number 0, as these are also falsy and would be removed by this check [2]. 2. Explicit Null Checks: Using if (variable!== null && variable!== undefined) or the more concise if (variable!= null) is the recommended way to explicitly guard against null and undefined [5][4]. This is safe, clear, and specifically targets the nullability issue without inadvertently excluding other valid values [5]. 3. Non-null Assertion Operator: The non-null assertion operator (!) is a postfix operator that tells the compiler to treat a value as non-null/non-undefined [5][6]. It does not perform a runtime check and is essentially an assertion by the developer [5][6]. Because it bypasses type checking, it should only be used when you are absolutely certain the value is present and the compiler cannot infer it [5][6]. Using it inside a conditional to "fix" a type error is generally discouraged because it creates a risk of runtime errors if your assumption is wrong [6]. In summary, instead of using a non-null assertion inside a conditional, you should use an explicit null check or a type guard to allow TypeScript to correctly narrow the type for you [5][4]. This leverages the language's safety features rather than overriding them [5].
Citations:
- 1: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0%2ehtml
- 2: https://www.typescriptlang.org/docs/handbook/2/narrowing.html
- 3: https://github.com/microsoft/TypeScript/wiki/Reference-Checker-Widening-Narrowing
- 4: https://basarat.gitbook.io/typescript/type-system/typeguard
- 5: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html
- 6: https://basarat.gitbook.io/typescript/intro/strictnullchecks
- 7: https://www.typescriptlang.org/docs/handbook/2/narrowing
Narrow status before reading its players.
online is a boolean, so {online && showPlayers && ...} does not narrow status from ServerStatus | null; status.players can still report as possibly null. Use optional chaining in the conditional and on the reads, or narrow status before the block.
🔧 Proposed fix
- {online && showPlayers && (
+ {status?.online && showPlayers && (
<span class="text-lum-text-secondary flex items-center gap-1.5">
<Users size={14} />
{status.players.online.toLocaleString()}
<span class="opacity-50">
- / {status.players.max.toLocaleString()}
+ / {status.max.toLocaleString()}🤖 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/ServerList/StatusBadge.tsx` around lines 13 - 39, Update the
players display condition in the StatusBadge component so it narrows status
before accessing player data, and ensure the status.players.online and
status.players.max reads remain safe when status or players is null. Preserve
the existing rendering behavior for online servers with showPlayers enabled.
| if (window.turnstile) { | ||
| render(); | ||
| } else { | ||
| // Load the script once; reuse it across widgets. | ||
| let script = document.querySelector<HTMLScriptElement>( | ||
| `script[src="${SCRIPT_SRC}"]` | ||
| ); | ||
| if (!script) { | ||
| script = document.createElement('script'); | ||
| script.src = SCRIPT_SRC; | ||
| script.async = true; | ||
| document.head.appendChild(script); | ||
| } | ||
| script.addEventListener('load', render); | ||
| } | ||
|
|
||
| ctx.cleanup(() => { | ||
| if (widgetId && window.turnstile) window.turnstile.remove(widgetId); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove the load listener on cleanup, and skip rendering after unmount.
If the component unmounts before the Turnstile script loads, the load listener still fires and calls render(). The widget is then created after ctx.cleanup ran, so window.turnstile.remove is never called for it. The listener also keeps render and the container reference alive.
🔧 Proposed fix
let widgetId: string | undefined;
+ let disposed = false;
const render = () => {
- if (!window.turnstile || !containerRef.value) return;
+ if (disposed || !window.turnstile || !containerRef.value) return;
widgetId = window.turnstile.render(containerRef.value, {
@@
- script.addEventListener('load', render);
+ script.addEventListener('load', render, { once: true });
}
ctx.cleanup(() => {
+ disposed = true;
+ script?.removeEventListener('load', render);
if (widgetId && window.turnstile) window.turnstile.remove(widgetId);
});Declare script outside the else branch so cleanup can reference it.
🤖 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/ServerList/Turnstile.tsx` around lines 57 - 76, Update the
Turnstile effect around render and cleanup so the script element and
load-handler state are accessible to ctx.cleanup. Remove the load listener
during cleanup and guard render with an active-mounted flag, ensuring the script
cannot create a widget after unmount while preserving normal immediate and
script-load rendering.
| try { | ||
| const body = new FormData(); | ||
| body.append('secret', secret); | ||
| body.append('response', token); | ||
| if (remoteIp) body.append('remoteip', remoteIp); | ||
|
|
||
| const res = await fetch(VERIFY_URL, { method: 'POST', body }); | ||
| const data: TurnstileVerifyResponse = await res.json(); | ||
|
|
||
| if (data.success) return { success: true }; | ||
| return { | ||
| success: false, | ||
| error: `CAPTCHA verification failed${data['error-codes']?.length ? `: ${data['error-codes'].join(', ')}` : ''}`, | ||
| }; | ||
| } catch (err) { | ||
| console.error('Turnstile verification error:', err); | ||
| return { success: false, error: 'CAPTCHA verification error' }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching turnstile/votifier:"
fd -a 'turnstile\.ts|votifier\.ts' . | sed 's#^\./##'
echo
echo "Relevant snippets:"
for f in $(fd 'turnstile\.ts|votifier\.ts' .); do
echo "--- $f ($(wc -l < "$f") lines) ---"
sed -n '1,220p' "$f"
done
echo
echo "Search for withTimeout and turnstile verify:"
rg -n "withTimeout|verifyTurnstile|TurnstileVerifyResponse|VERIFY_URL|voteForServer|challenges.cloudflare.com|turnstile" . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**'Repository: birdflop/web
Length of output: 11148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
targets = sorted(Path('.').glob('**/*.ts'))
turnstile_code = None
for path in targets:
text = path.read_text(errors='ignore')
if path.as_posix().endswith('src/util/serverlist/turnstile.ts'):
turnstile_code = text
continue
if not turnstile_code:
print("turnstile.ts not found")
raise SystemExit
checks = {
"fetch_without_signal": "const res = await fetch(VERIFY_URL, { method: 'POST', body });" in turnstile_code,
"fetch_has_signal": bool(re.search(r"fetch\(VERIFY_URL\s*,\s*{[\s\S]*?signal:", turnstile_code)),
"verify_returns_on_success": "if (data.success) return { success: true };" in turnstile_code,
"returns_success_false_on_failure": "return { success: false, error:" in turnstile_code,
}
for k, v in checks.items():
print(f"{k}={v}")
PYRepository: birdflop/web
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- src/util/serverlist/actions.ts lines 150-190 ---"
sed -n '150,190p' src/util/serverlist/actions.ts
echo
echo "--- package/dev tool clues ---"
for f in package.json vite.config.* qwik*.json .github/workflows/*; do
[ -f "$f" ] && echo "### $f" && sed -n '1,220p' "$f"
doneRepository: birdflop/web
Length of output: 16662
Add a timeout to the Turnstile verification fetch.
voteForServer awaits verifyTurnstile() on the server action request path. If challenges.cloudflare.com/turnstile/v0/siteverify does not respond, this fetch can hold the vote request until the outer platform timeout runs and no Votifier delivery is attempted.
src/util/serverlist/votifier.ts already defines a 5s timeout for external socket operations; use the same timeout discipline for this HTTP call, including AbortController.abort() plus clearing the timer.
🤖 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/util/serverlist/turnstile.ts` around lines 27 - 44, Update the Turnstile
verification fetch in verifyTurnstile to use an AbortController with a 5-second
timeout, passing its signal to fetch and aborting when the timer expires. Clear
the timeout after the fetch completes or fails, while preserving the existing
success, failure, and catch handling.
| function parsePort( | ||
| value: string | number | undefined | ||
| ): number | null | undefined { | ||
| if (value === undefined || value === '' || value === null) return null; | ||
| const n = typeof value === 'number' ? value : parseInt(value, 10); | ||
| if (!Number.isInteger(n)) return undefined; // signal "invalid" | ||
| if (n < 1 || n > 65535) return undefined; | ||
| return n; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject port strings with trailing non-numeric characters.
Line 79 accepts 25565abc as 25565 and 1.5 as 1. validateServerInput is the persistence boundary, so malformed input is stored as a different port. Require the complete trimmed string to match decimal digits before converting it with Number().
Proposed fix
- if (value === undefined || value === '' || value === null) return null;
- const n = typeof value === 'number' ? value : parseInt(value, 10);
+ if (value === undefined || value === null) return null;
+ if (typeof value === 'string' && value.trim() === '') return null;
+ if (typeof value === 'string' && !/^\d+$/.test(value.trim()))
+ return undefined;
+ const n = typeof value === 'number' ? value : Number(value.trim());📝 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 parsePort( | |
| value: string | number | undefined | |
| ): number | null | undefined { | |
| if (value === undefined || value === '' || value === null) return null; | |
| const n = typeof value === 'number' ? value : parseInt(value, 10); | |
| if (!Number.isInteger(n)) return undefined; // signal "invalid" | |
| if (n < 1 || n > 65535) return undefined; | |
| return n; | |
| function parsePort( | |
| value: string | number | undefined | |
| ): number | null | undefined { | |
| if (value === undefined || value === null) return null; | |
| if (typeof value === 'string' && value.trim() === '') return null; | |
| if (typeof value === 'string' && !/^\d+$/.test(value.trim())) | |
| return undefined; | |
| const n = typeof value === 'number' ? value : Number(value.trim()); | |
| if (!Number.isInteger(n)) return undefined; // signal "invalid" | |
| if (n < 1 || n > 65535) return undefined; | |
| return n; |
🤖 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/util/serverlist/validation.ts` around lines 75 - 82, Update parsePort to
validate string inputs after trimming, requiring the complete value to consist
only of decimal digits before conversion. Replace the partial parseInt behavior
with Number() after this check, while preserving existing undefined, empty,
null, integer, and 1–65535 range handling.
| // Hostnames: allow domain names and IPv4 (no scheme, no path). | ||
| const HOST_RE = /^[a-zA-Z0-9.-]+$/; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/util/serverlist/status.ts --items all
ast-grep outline src/util/serverlist/votifier.ts --items all
rg -n -C 3 --type ts \
'fetch\s*\(|https?\.request|net\.connect|createConnection|dgram|javaHost|bedrockHost|votifierHost' \
src/util/serverlist/status.ts \
src/util/serverlist/votifier.ts \
src/util/serverlist/actions.tsRepository: birdflop/web
Length of output: 3850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)validation\.ts$|(^|/)status\.ts$|(^|/)votifier\.ts$|(^|/)actions\.ts$' || true
echo "== validation outline and relevant lines =="
sed -n '1,150p' src/util/serverlist/validation.ts
echo "== status.ts =="
cat -n src/util/serverlist/status.ts
echo "== votifier.ts =="
cat -n src/util/serverlist/votifier.ts
echo "== status/votifier imports and usages =="
rg -n -C 2 'validate.*host|host.*valid|validation|fetchStatus|sendVotifierV2|javaHost|bedrockHost|votifierHost|localhost|127\.0\.0\.1|private|reserved' src || trueRepository: birdflop/web
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const HOST_RE = /^[a-zA-Z0-9.-]+$/;
const values = ['localhost', '127.0.0.1', '192.168.1.10', '10.0.0.1', '172.16.0.1', '169.254.1.1', '0.0.0.0'];
for (const v of values) {
console.log(v, HOST_RE.test(v));
}
try {
console.log('dns.lookup localhost', JSON.stringify(require('node:dns').lookupSync('localhost', { family: 0 }), null, 2));
} catch (e) {
console.log('dns.resolve localhost', JSON.stringify(require('node:dns').resolveSync('localhost'), null, 2));
}
JSRepository: birdflop/web
Length of output: 1018
Block private or reserved destinations before checking status or sending Votifier messages.
HOST_RE permits localhost, 127.0.0.1, private RFC 1918 ranges, link-local addresses, and 0.0.0.0. status.ts passes those values to fetch() for mcstatus, while sendVotifierV2() opens cloudflare:sockets using the stored Votifier host. Resolve hostnames before checking status, reject private/reserved destinations, and block reserved IPs before creating the Votifier TCP connection.
🤖 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/util/serverlist/validation.ts` around lines 94 - 95, Update server
destination validation around HOST_RE to resolve hostnames and reject private or
reserved IP ranges, including localhost, loopback, RFC 1918, link-local,
unspecified, and equivalent IPv6 addresses, before any status check or Votifier
message is sent. Ensure status.ts validates the resolved destination before
fetch() and sendVotifierV2() performs the same protection before creating the
cloudflare:sockets TCP connection.
| let socket: | ||
| | { | ||
| readable: ReadableStream; | ||
| writable: WritableStream; | ||
| close: () => Promise<void>; | ||
| } | ||
| | undefined; | ||
|
|
||
| try { | ||
| // Dynamically imported so this module never gets pulled into a client bundle. | ||
| const { connect } = (await import('cloudflare:sockets')) as { | ||
| connect: (addr: { hostname: string; port: number }) => { | ||
| readable: ReadableStream; | ||
| writable: WritableStream; | ||
| close: () => Promise<void>; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Type the socket streams to fix the no-unsafe-argument warnings.
socket.readable/socket.writable and the connect return type use bare ReadableStream/WritableStream, so reader.read() returns any for .value. That any value later flows into TextDecoder.decode() at Lines 108 and 142-144, which is what CI flags.
Type the streams as ReadableStream<Uint8Array>/WritableStream<Uint8Array> here, at the declaration sites, instead of casting at each call site.
🔧 Proposed fix
let socket:
| {
- readable: ReadableStream;
- writable: WritableStream;
+ readable: ReadableStream<Uint8Array>;
+ writable: WritableStream<Uint8Array>;
close: () => Promise<void>;
}
| undefined; const { connect } = (await import('cloudflare:sockets')) as {
connect: (addr: { hostname: string; port: number }) => {
- readable: ReadableStream;
- writable: WritableStream;
+ readable: ReadableStream<Uint8Array>;
+ writable: WritableStream<Uint8Array>;
close: () => Promise<void>;
};
};As per pipeline failures, "vp check reported TypeScript no-unsafe-argument: values of type any are passed to TextDecoder.decode."
📝 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.
| let socket: | |
| | { | |
| readable: ReadableStream; | |
| writable: WritableStream; | |
| close: () => Promise<void>; | |
| } | |
| | undefined; | |
| try { | |
| // Dynamically imported so this module never gets pulled into a client bundle. | |
| const { connect } = (await import('cloudflare:sockets')) as { | |
| connect: (addr: { hostname: string; port: number }) => { | |
| readable: ReadableStream; | |
| writable: WritableStream; | |
| close: () => Promise<void>; | |
| }; | |
| }; | |
| let socket: | |
| | { | |
| readable: ReadableStream<Uint8Array>; | |
| writable: WritableStream<Uint8Array>; | |
| close: () => Promise<void>; | |
| } | |
| | undefined; | |
| try { | |
| // Dynamically imported so this module never gets pulled into a client bundle. | |
| const { connect } = (await import('cloudflare:sockets')) as { | |
| connect: (addr: { hostname: string; port: number }) => { | |
| readable: ReadableStream<Uint8Array>; | |
| writable: WritableStream<Uint8Array>; | |
| close: () => Promise<void>; | |
| }; | |
| }; |
🤖 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/util/serverlist/votifier.ts` around lines 75 - 91, Update the socket
declaration and the dynamically imported connect return type in the votifier
flow to use ReadableStream<Uint8Array> and WritableStream<Uint8Array> instead of
unparameterized stream types, ensuring reader.read().value is typed for
TextDecoder.decode without call-site casts.
Source: Pipeline failures
Auto-detect listings hosted on Birdflop by resolving their address (DNS-over-HTTPS, incl. Java SRV) against panel node/allocation IPs pulled from the Pterodactyl Application API and cached in D1 - no hardcoded IPs, nothing exposed to clients beyond a boolean badge. Re-checked on edit and lazily once a day on detail views. Also fixes issues found while auditing the feature: - strip Votifier host/port/token from public loader payloads (the token allowed forging votes straight to a server's NuVotifier port) - gate the edit loader so non-owners no longer receive the full row - return only name/image for listing owners instead of the whole user row (leaked emails) - require http(s) URLs for the Discord link (javascript: URI XSS) - set createdAt explicitly on inserts; the CURRENT_TIMESTAMP default stores text, making vote cooldowns permanent and monthly counts all-time (text sorts above numbers in SQLite) - cap listings at 10 per account, one report per server per user/IP per 24h, and guard against NaN page params
Deploying web with
|
| Latest commit: |
449a129
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.env.example:
- Line 11: Add a trailing blank line after the PANEL_URL entry in the
environment template so the file ends with the required blank line and satisfies
dotenv-linter’s EndingBlankLine check.
In `@drizzle/migrations/meta/0012_snapshot.json`:
- Line 1: Run vp check --fix to regenerate the formatting for the
0012_snapshot.json generated snapshot, then include the resulting file changes
in the commit.
In `@src/util/serverlist/birdflop.ts`:
- Around line 50-53: Update both dohQuery() and fetchPanelIps() to apply the
project’s existing abort/timeout policy to their external fetch calls, ensuring
neither request can remain unbounded. Preserve fetchPanelIps()’s stale IP-cache
fallback when the timed request fails.
🪄 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 Plus
Run ID: 300fe797-29d6-4152-8e7f-98585e41f012
📒 Files selected for processing (14)
.env.exampledrizzle/migrations/0012_black_demogoblin.sqldrizzle/migrations/meta/0012_snapshot.jsondrizzle/migrations/meta/_journal.jsondrizzle/schema.tssrc/components/ServerList/ServerCard.tsxsrc/routes/serverlist/[slug]/edit/index.tsxsrc/routes/serverlist/[slug]/index.tsxsrc/routes/serverlist/index.tsxsrc/util/serverlist/actions.tssrc/util/serverlist/birdflop.tssrc/util/serverlist/constants.tssrc/util/serverlist/queries.tssrc/util/serverlist/validation.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/util/serverlist/constants.ts
- src/routes/serverlist/[slug]/edit/index.tsx
- src/routes/serverlist/index.tsx
- src/util/serverlist/actions.ts
- src/util/serverlist/validation.ts
- src/routes/serverlist/[slug]/index.tsx
- src/components/ServerList/ServerCard.tsx
| const res = await fetch( | ||
| `${DOH_URL}?name=${encodeURIComponent(name)}&type=${type}`, | ||
| { headers: { accept: 'application/dns-json' } } | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## repository files matching birdflop.ts"
fd -a 'birdflop\.ts$' . || true
echo
echo "## outline"
ast-grep outline src/util/serverlist/birdflop.ts --view expanded || true
echo
echo "## relevant file sections"
wc -l src/util/serverlist/birdflop.ts
sed -n '1,230p' src/util/serverlist/birdflop.ts | cat -n
echo
echo "## fetch/timeout/policy references"
rg -n "fetch\\(|RequestTimeout|timeout|AbortSignal|AbortController|setTimeout|Promise.race|shared|policy|DOH_URL|apiKey|panelUrl" src || trueRepository: birdflop/web
Length of output: 22749
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## caller sites in birdflop.ts"
sed -n '204,350p' src/util/serverlist/birdflop.ts | cat -n
echo
echo "## actions usages"
sed -n '30,90p' src/util/serverlist/actions.ts | cat -n
rg -n "detectBirdflopHosted|refreshBirdflopHosted|isBirdflopHosted|birdflop" src/util/serverlist/actions.ts src/routes src/components -g '*.ts' -g '*.tsx' | head -n 120Repository: birdflop/web
Length of output: 18483
🌐 Web query:
Cloudflare Workers request limit time budget timeout abortController fetch documentation
💡 Result:
Cloudflare Workers support the standard Fetch API and AbortController/AbortSignal for managing request timeouts [1][2]. ### Fetch Timeouts with AbortController To implement a timeout for a fetch request in a Cloudflare Worker, you use an AbortController to signal the cancellation of the fetch operation [1][2]. Example implementation: javascript export default { async fetch(request) { const controller = new AbortController(); // Set a timeout of 5000 milliseconds const timeoutId = setTimeout(() => controller.abort(), 5000); try { const response = await fetch("https://example.com", { signal: controller.signal }); return response; } catch (err) { if (err.name === 'AbortError') { return new Response("Request timed out", { status: 504 }); } throw err; } finally { clearTimeout(timeoutId); } } }; ### Key Considerations 1. CPU Time vs. Wall Time: Workers have strict CPU time limits (e.g., 10ms for free tier, 30s-5m for paid tiers) [3][4]. Importantly, time spent waiting for network requests (including fetch calls) does not count toward CPU time [3]. 2. Wall Time Limits: While there is no hard "wall time" limit for incoming HTTP requests (as long as the client remains connected), Cron Triggers, Queue consumers, and Durable Object alarms have a 15-minute wall time limit [4]. 3. AbortSignal Support: The AbortSignal is used to cancel pending asynchronous operations [2][5]. When a signal passed to fetch is aborted, the fetch promise rejects with an AbortError [5]. 4. Context: Ensure you handle the fetch operation within your worker's execution context [1][6]. If you need to perform work after the response is sent, use ctx.waitUntil() [4]. For detailed limits, see the official Cloudflare Workers Limits documentation [3].
Citations:
- 1: https://developers.cloudflare.com/workers/runtime-apis/request/
- 2: https://developers.cloudflare.com/workers/runtime-apis/web-standards/index.md
- 3: https://developers.cloudflare.com/workers/platform/limits/
- 4: https://developers.cloudflare.com/workers/platform/limits/index.md
- 5: https://developers.cloudflare.com/workers/runtime-apis/scheduler/
- 6: https://developers.cloudflare.com/workers/runtime-apis/handlers/fetch/
Add deadlines to Birdflop external requests.
dohQuery() and fetchPanelIps() make unbounded Cloudflare fetch() calls, and these paths run during server creation plus detail-page refreshes. Add the same abort/timeout policy to both calls. The stale IP-cache fallback should remain for the panel refresh.
📍 Affects 1 file
src/util/serverlist/birdflop.ts#L50-L53(this comment)src/util/serverlist/birdflop.ts#L168-L176
🤖 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/util/serverlist/birdflop.ts` around lines 50 - 53, Update both dohQuery()
and fetchPanelIps() to apply the project’s existing abort/timeout policy to
their external fetch calls, ensuring neither request can remain unbounded.
Preserve fetchPanelIps()’s stale IP-cache fallback when the timed request fails.
…into feat/add-serverlist
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/components/rgbirdflop/presets/PresetPreview.tsx`:
- Around line 48-55: Update getGradient to return undefined when colors is
absent or has length zero, before constructing the linear-gradient string.
Preserve the existing gradient generation for non-empty ColorStop arrays.
In `@src/routes/resources/rgb/presets/index.tsx`:
- Around line 442-445: The default perPage value of 20 is missing from both
pagination controls. Update the Pagination instances at
src/routes/resources/rgb/presets/index.tsx lines 442-445 and 474-477 to include
20 in their perPageOptions lists, keeping both controls consistent.
🪄 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 Plus
Run ID: 1dccde48-8f54-41ef-89ca-3e3d44e3876e
📒 Files selected for processing (10)
drizzle/migrations/meta/0012_snapshot.jsondrizzle/migrations/meta/_journal.jsonsrc/components/Elements/MotdText.tsxsrc/components/Elements/Output.tsxsrc/components/Elements/Pagination.tsxsrc/components/ServerList/ServerCard.tsxsrc/components/rgbirdflop/presets/PresetPreview.tsxsrc/routes/resources/rgb/presets/index.tsxsrc/routes/serverlist/[slug]/index.tsxsrc/routes/serverlist/index.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- drizzle/migrations/meta/_journal.json
- src/components/Elements/Output.tsx
- drizzle/migrations/meta/0012_snapshot.json
- src/components/ServerList/ServerCard.tsx
- src/routes/serverlist/[slug]/index.tsx
- src/routes/serverlist/index.tsx
| export function getGradient(colors?: ColorStop[]) { | ||
| if (!colors) { | ||
| return undefined; | ||
| } | ||
| return `linear-gradient(to bottom right, ${colors | ||
| ?.map((color) => `${color.hex}10 ${color.pos}%`) | ||
| .join(', ')})`; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle empty color arrays.
ColorStop[] permits []. The current code then generates linear-gradient(to bottom right, ), which is invalid CSS. Return undefined when colors.length === 0.
Proposed fix
export function getGradient(colors?: ColorStop[]) {
- if (!colors) {
+ if (!colors?.length) {
return undefined;
}📝 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 getGradient(colors?: ColorStop[]) { | |
| if (!colors) { | |
| return undefined; | |
| } | |
| return `linear-gradient(to bottom right, ${colors | |
| ?.map((color) => `${color.hex}10 ${color.pos}%`) | |
| .join(', ')})`; | |
| } | |
| export function getGradient(colors?: ColorStop[]) { | |
| if (!colors?.length) { | |
| return undefined; | |
| } | |
| return `linear-gradient(to bottom right, ${colors | |
| ?.map((color) => `${color.hex}10 ${color.pos}%`) | |
| .join(', ')})`; | |
| } |
🤖 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/rgbirdflop/presets/PresetPreview.tsx` around lines 48 - 55,
Update getGradient to return undefined when colors is absent or has length zero,
before constructing the linear-gradient string. Preserve the existing gradient
generation for non-empty ColorStop arrays.
| totalCount={presetCount} | ||
| rowsLength={publicPresets.length} | ||
| itemLabel="presets" | ||
| perPageOptions={[12, 24, 48, 96]} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline src/components/Elements/Pagination.tsx --items all
rg -n -C 6 'perPageOptions|perPage|<select|option' \
src/components/Elements/Pagination.tsx \
src/routes/resources/rgb/presets/index.tsxRepository: birdflop/web
Length of output: 14266
Keep the default perPage selectable in both pagination controls.
perPage defaults to 20, but both <Pagination> instances pass [12, 24, 48, 96]. When no perPage URL parameter is present, the page renders 20 results while the per-page options do not include the active selection. Include 20 in both perPageOptions lists, or change the loader default to an option that is included.
📍 Affects 1 file
src/routes/resources/rgb/presets/index.tsx#L442-L445(this comment)src/routes/resources/rgb/presets/index.tsx#L474-L477
🤖 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/routes/resources/rgb/presets/index.tsx` around lines 442 - 445, The
default perPage value of 20 is missing from both pagination controls. Update the
Pagination instances at src/routes/resources/rgb/presets/index.tsx lines 442-445
and 474-477 to include 20 in their perPageOptions lists, keeping both controls
consistent.
…t directly into the plugin configuration workflow
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/util/motd.ts (1)
154-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate spread-hex parsing becomes unreachable after normalization.
normalizeMotdTextnow converts spread hex (&x&1&2.../§x§1§2...) and compact hex (&x123456) into§#RRGGBBformat beforeparseMotdLinereads the text (Line 156). The char-loop inparseMotdLinestill contains its own spread-hex branch (next === 'x'handling further down in this function) that duplicates this conversion. For any well-formed spread/compact hex input, that branch is now dead code, since the pattern is already rewritten to§#RRGGBBbefore the loop runs.Consider removing the now-redundant spread-hex branch in the char loop, or add a comment explaining that it remains only as a fallback for inputs
normalizeMotdTextdoes not fully match.🤖 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/util/motd.ts` around lines 154 - 156, Remove the redundant spread-hex handling branch triggered by next === 'x' from parseMotdLine’s character loop, since normalizeMotdText already converts supported spread and compact hex formats before parsing. If the branch must remain for unmatched inputs, document that it is an intentional fallback rather than duplicate normal processing.
🤖 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/routes/resources/plugins/index.tsx`:
- Around line 173-195: Update the “My Server” deletion guard in the user-server
merge loop so it deletes the placeholder only when its plugins are empty and its
serverId is not already set. Preserve linked entries while retaining the
existing behavior for unlinked placeholders.
- Around line 65-104: Add a fixed timeout around each getServerStatus call in
useUserServers, using an AbortController or Promise.race so stalled mcstatus.io
requests cannot keep the route loader open. Preserve the existing per-server
fallback of icon: null when the status fetch times out or fails, and continue
resolving other servers independently.
In `@src/util/motd.ts`:
- Around line 89-137: Update the MOTD conversion logic around the chained
replacements and generic tag handler to maintain a style stack for opening and
closing span tags: push the current formatting when a span applies color or text
styles, and restore the matching prior style when its closing tag is encountered
instead of always emitting §r. Handle unsupported or differently formatted span
attributes without emitting an unmatched reset, while preserving recognized
MiniMessage tag conversions and existing formatting behavior.
- Around line 154-156: Update the MotdText line-splitting flow to normalize <br>
tags before splitting on newline characters, or normalize each segment before
renderMotdLine is called. Ensure embedded <br> values produce separate rendered
rows rather than remaining as newline characters within one MotdRun segment,
while preserving existing newline handling.
---
Nitpick comments:
In `@src/util/motd.ts`:
- Around line 154-156: Remove the redundant spread-hex handling branch triggered
by next === 'x' from parseMotdLine’s character loop, since normalizeMotdText
already converts supported spread and compact hex formats before parsing. If the
branch must remain for unmatched inputs, document that it is an intentional
fallback rather than duplicate normal processing.
🪄 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 Plus
Run ID: 04851e79-dd4a-40af-b17a-1d44096e803d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.jsonpnpm-workspace.yamlsrc/components/ServerList/ServerCard.tsxsrc/components/ServerList/ServerControls.tsxsrc/components/rgbirdflop/presets/MyPrivatePresets.tsxsrc/routes/resources/plugins/index.tsxsrc/routes/serverlist/[slug]/index.tsxsrc/util/motd.tssrc/util/serverlist/status.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/util/serverlist/status.ts
- src/components/ServerList/ServerCard.tsx
- src/components/rgbirdflop/presets/MyPrivatePresets.tsx
- src/components/ServerList/ServerControls.tsx
- src/routes/serverlist/[slug]/index.tsx
| if (userServers.value.length > 0) { | ||
| for (const us of userServers.value) { | ||
| const existingKey = Object.keys(initialServers).find( | ||
| (k) => | ||
| initialServers[k].serverId === us.id || | ||
| k.toLowerCase() === us.name.toLowerCase() | ||
| ); | ||
| if (existingKey) { | ||
| initialServers[existingKey].serverId = us.id; | ||
| } else { | ||
| if ( | ||
| initialServers['My Server'] && | ||
| Object.keys(initialServers['My Server'].plugins).length === 0 | ||
| ) { | ||
| delete initialServers['My Server']; | ||
| } | ||
| initialServers[us.name] = { | ||
| ...serverDefaults, | ||
| serverId: us.id, | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fix the 'My Server' deletion check to preserve already-linked entries.
The deletion guard only checks that initialServers['My Server'].plugins is empty. It does not check whether initialServers['My Server'] already has a serverId. Consider this sequence:
dbPlugins.servershas{ 'My Server': { ..., serverId: server1.id, plugins: {} } }(server1 already linked, no plugins added yet).- The user owns a second listed server,
server2, that does not match'My Server'by id or name.
On the server1 iteration, existingKey matches 'My Server' and keeps it. On the server2 iteration, existingKey finds no match, so the code falls into the else branch, sees 'My Server' still has empty plugins, and deletes it — losing server1's linked entry entirely. Since pluginsStore state is persisted on the following autosave via setUserData, this loss becomes permanent, not just a render artifact.
Check serverId before deleting the placeholder.
🐛 Proposed fix to avoid deleting an already-linked entry
} else {
if (
initialServers['My Server'] &&
+ !initialServers['My Server'].serverId &&
Object.keys(initialServers['My Server'].plugins).length === 0
) {
delete initialServers['My Server'];
}
initialServers[us.name] = {
...serverDefaults,
serverId: us.id,
};
}📝 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 (userServers.value.length > 0) { | |
| for (const us of userServers.value) { | |
| const existingKey = Object.keys(initialServers).find( | |
| (k) => | |
| initialServers[k].serverId === us.id || | |
| k.toLowerCase() === us.name.toLowerCase() | |
| ); | |
| if (existingKey) { | |
| initialServers[existingKey].serverId = us.id; | |
| } else { | |
| if ( | |
| initialServers['My Server'] && | |
| Object.keys(initialServers['My Server'].plugins).length === 0 | |
| ) { | |
| delete initialServers['My Server']; | |
| } | |
| initialServers[us.name] = { | |
| ...serverDefaults, | |
| serverId: us.id, | |
| }; | |
| } | |
| } | |
| } | |
| if (userServers.value.length > 0) { | |
| for (const us of userServers.value) { | |
| const existingKey = Object.keys(initialServers).find( | |
| (k) => | |
| initialServers[k].serverId === us.id || | |
| k.toLowerCase() === us.name.toLowerCase() | |
| ); | |
| if (existingKey) { | |
| initialServers[existingKey].serverId = us.id; | |
| } else { | |
| if ( | |
| initialServers['My Server'] && | |
| !initialServers['My Server'].serverId && | |
| Object.keys(initialServers['My Server'].plugins).length === 0 | |
| ) { | |
| delete initialServers['My Server']; | |
| } | |
| initialServers[us.name] = { | |
| ...serverDefaults, | |
| serverId: us.id, | |
| }; | |
| } | |
| } | |
| } |
🤖 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/routes/resources/plugins/index.tsx` around lines 173 - 195, Update the
“My Server” deletion guard in the user-server merge loop so it deletes the
placeholder only when its plugins are empty and its serverId is not already set.
Preserve linked entries while retaining the existing behavior for unlinked
placeholders.
| text = text | ||
| .replace(/<br\s*\/?>/gi, '\n') | ||
| .replace( | ||
| /<span[^>]*style="[^"]*color:\s*#([0-9a-fA-F]{6})[^"]*"[^>]*>/gi, | ||
| '§#$1' | ||
| ) | ||
| .replace( | ||
| /<span[^>]*style="[^"]*color:\s*rgb\((\d+),\s*(\d+),\s*(\d+)\)[^"]*"[^>]*>/gi, | ||
| (_, r, g, b) => { | ||
| const hex = [r, g, b] | ||
| .map((x) => | ||
| Math.min(255, Math.max(0, parseInt(x, 10))) | ||
| .toString(16) | ||
| .padStart(2, '0') | ||
| ) | ||
| .join(''); | ||
| return `§#${hex}`; | ||
| } | ||
| ) | ||
| .replace( | ||
| /<span[^>]*class="[^"]*motd-color-([0-9a-fA-F]{6})[^"]*"[^>]*>/gi, | ||
| '§#$1' | ||
| ) | ||
| .replace(/<span[^>]*style="[^"]*font-weight:\s*bold[^"]*"[^>]*>/gi, '§l') | ||
| .replace(/<span[^>]*style="[^"]*font-style:\s*italic[^"]*"[^>]*>/gi, '§o') | ||
| .replace(/<\/span>/gi, '§r') | ||
| .replace(/<[^>]+>/g, (tag) => { | ||
| const hexMatch = tag.match(/^<(?:color:|c:)?#([0-9a-fA-F]{6})>$/i); | ||
| if (hexMatch) return `§#${hexMatch[1]}`; | ||
|
|
||
| const miniMsgTags: Record<string, string> = { | ||
| '</color>': '§r', | ||
| '<reset>': '§r', | ||
| '<r>': '§r', | ||
| '<bold>': '§l', | ||
| '<b>': '§l', | ||
| '<italic>': '§o', | ||
| '<i>': '§o', | ||
| '<underlined>': '§n', | ||
| '<u>': '§n', | ||
| '<strikethrough>': '§m', | ||
| '<st>': '§m', | ||
| '<obfuscated>': '§k', | ||
| '<obf>': '§k', | ||
| }; | ||
| const lower = tag.toLowerCase(); | ||
| if (lower in miniMsgTags) return miniMsgTags[lower]; | ||
| return ''; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Closing </span> performs a full reset instead of restoring the previous style.
Every </span> maps unconditionally to §r at Line 114. In parseMotdLine, §r resets all formatting, including color. For nested spans (for example, bold text inside a colored span), closing the inner span erases the outer color instead of just clearing bold. HTML sources that nest formatting spans render incorrectly after the nested tag closes.
Separately, the generic tag handler at Lines 115-137 silently drops any tag it does not recognize (for example, <span style="color: red;"> with a named CSS color, or attributes in a different order/case). The opening tag disappears with no color fallback, and the paired </span> still emits a stray §r.
A correct fix needs a style stack: push the current style when an opening tag applies a change, and pop back to the prior style on the matching closing tag, instead of a single global §r.
🤖 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/util/motd.ts` around lines 89 - 137, Update the MOTD conversion logic
around the chained replacements and generic tag handler to maintain a style
stack for opening and closing span tags: push the current formatting when a span
applies color or text styles, and restore the matching prior style when its
closing tag is encountered instead of always emitting §r. Handle unsupported or
differently formatted span attributes without emitting an unmatched reset, while
preserving recognized MiniMessage tag conversions and existing formatting
behavior.
| // Parse a single MOTD line (using & or § codes, hex codes, or HTML) into styled runs. | ||
| export function parseMotdLine(rawLine: string): MotdRun[] { | ||
| const line = normalizeMotdText(rawLine); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Trace how MOTD text reaches parseMotdLine/renderMotdLine and whether it is pre-split by newline.
rg -n -C5 'renderMotdLine|parseMotdLine|normalizeMotdText' src/components/Elements/MotdText.tsx src/util/motd.tsRepository: birdflop/web
Length of output: 3381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== MotdText outline =="
ast-grep outline src/components/Elements/MotdText.tsx || true
echo "== MotdText relevant sections =="
sed -n '1,90p' src/components/Elements/MotdText.tsx | cat -n
echo "== motd parseLine relevant sections =="
sed -n '80,240p' src/util/motd.ts | cat -n
echo "== renderMotdLine call sites =="
rg -n -C3 'renderMotdLine\s*\(' srcRepository: birdflop/web
Length of output: 10372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all parseMotdLine/renderMotdLine usages =="
rg -n -C2 'parseMotdLine\s*\(|renderMotdLine\s*\(' src
echo "== MotdText surrounding output =="
sed -n '44,54p' src/components/Elements/MotdText.tsx | cat -n
echo "== inspect parseMotdLine loop around newlines =="
sed -n '76,180p' src/util/motd.ts | cat -nRepository: birdflop/web
Length of output: 6731
Process <br> before MotdText splits lines.
MotdText calls text.split(/\r?\n/), then renders each segment with renderMotdLine(parsed line). An embedded <br> is converted to \n by normalizeMotdText, so a single segment can contain an invisible line-break character instead of producing a second <div> row. Move or duplicate <br> normalization before split(/\r?\n/), or normalize again inside lines.map.
🤖 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/util/motd.ts` around lines 154 - 156, Update the MotdText line-splitting
flow to normalize <br> tags before splitting on newline characters, or normalize
each segment before renderMotdLine is called. Ensure embedded <br> values
produce separate rendered rows rather than remaining as newline characters
within one MotdRun segment, while preserving existing newline handling.
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
packages/mc-status/src/motd/parser.ts (1)
338-349: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe array branch is unreachable.
typeof input === 'object'is true for arrays, so line 340 always matches first and lines 342-343 never run. The behavior is still correct becauseparseChatComponentchecksArray.isArrayat line 269. Remove the dead branch to avoid confusion.♻️ Proposed cleanup
if (!input) { runs = [[]]; - } else if (typeof input === 'object' && input !== null) { - runs = parseChatComponent(input); - } else if (Array.isArray(input)) { - runs = parseChatComponent(input as (string | ChatComponent)[]); + } else if (typeof input === 'object') { + runs = parseChatComponent( + input as ChatComponent | (string | ChatComponent)[] + ); } else if (typeof input === 'string') {🤖 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 `@packages/mc-status/src/motd/parser.ts` around lines 338 - 349, Remove the unreachable Array.isArray branch from the input handling near parseMotdLine; arrays are already handled by the preceding object branch through parseChatComponent. Preserve the existing handling for nullish, object, string, and fallback inputs.packages/mc-status/tsconfig.json (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude the test directory from the build.
includecoverssrc/__tests__/status.test.ts.pnpm buildrunstsc, so the compiler must resolve thevitestimport, and the emitted test files land inlibandlib-types, whichpackage.jsonpublishes. Add anexcludeentry.♻️ Proposed change
- "include": ["src/**/*"] + "include": ["src/**/*"], + "exclude": ["src/**/__tests__/**"]🤖 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 `@packages/mc-status/tsconfig.json` at line 17, Update the packages/mc-status tsconfig include/exclude configuration to exclude the src/__tests__ directory from TypeScript compilation, preventing test files from being emitted into lib and lib-types while leaving the existing source inclusion unchanged.packages/mc-status/package.json (1)
18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a lower Node engine floor.
The implementation uses only
node:dgram,node:net, andnode:dns. A>=24.13.0floor blocks consumers on Node 20 and 22 for no functional reason. If the floor is intentional, keep it; otherwise lower it to widen adoption of the published package.🤖 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 `@packages/mc-status/package.json` around lines 18 - 20, Review the engines.node requirement in package.json and lower the minimum supported Node.js version from >=24.13.0 to the lowest compatible floor, preferably covering Node 20 and 22, since the package only relies on node:dgram, node:net, and node:dns. Keep the current floor only if repository compatibility requirements intentionally mandate Node 24.13.0.
🤖 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 `@packages/mc-status/package.json`:
- Around line 7-13: Update the package exports entry for "." to remove the
separate "require" condition and use the "types" condition as the runtime
exports entry, preserving the supported runtime condition order and keeping the
declaration path last only when it is a valid resolution branch.
In `@packages/mc-status/src/protocol/bedrock.ts`:
- Line 6: Update the RAKNET_MAGIC constant to use the standard RakNet magic
bytes 00ffff00fefefefefdfdfdfd12345678, ensuring Bedrock ID_UNCONNECTED_PING
packets are accepted and fetchStatus avoids the offline fallback.
In `@packages/mc-status/src/protocol/java.ts`:
- Around line 96-161: Update the data handler around readVarInt and its catch
path to distinguish incomplete-buffer underflow from fatal protocol errors. Use
a dedicated exported IncompleteBufferError from varint.ts (and have relevant
parsing helpers throw it), leave receivedBuffer intact and return when that
error occurs, and continue calling fail() for all other errors.
In `@src/routes/resources/status/index.tsx`:
- Around line 17-45: Validate the parsed port in useStatusChecker before passing
it to fetchStatus: accept only a finite integer in the range 1–65535, and
normalize missing, non-numeric, fractional, or out-of-range values to null so
fetchStatus can apply its default-port behavior.
- Around line 82-158: Add protection to the status-pinging flow behind the
form’s handleSearch submission and fetchStatus call: enforce server-side rate
limiting or bot verification before allowing requests using the user-supplied
host, edition, and port. Reject excess or unverified requests without opening
TCP/UDP connections, while preserving normal status checks for permitted users.
In `@src/util/motd.ts`:
- Around line 154-160: Replace the local implementations of isCustomHexColor,
normalizeMotdText, and parseMotdLine in src/util/motd.ts with imports from the
published `@birdflop/mc-status` package, then remove the duplicated helper code
and any now-unused dependencies while preserving the existing callers and
exports.
- Around line 162-171: The duplicated MOTD parser must be removed in favor of
the shared implementation. In src/util/motd.ts:154-160, import isCustomHexColor,
normalizeMotdText, and parseMotdLine from `@birdflop/mc-status` and delete the
local copies; in src/util/motd.ts:162-171, no direct change is needed because
the local parseMotdLine is removed and the shared parser already resets style
and recomputes isExplicit correctly.
In `@src/util/serverlist/status.ts`:
- Around line 71-89: Add latency propagation to fetchStatus and the ServerStatus
type in packages/mc-status/src/types.ts, then render it in the results card near
players/version in src/routes/resources/status/index.tsx at lines 76-79; update
the meta description at lines 199-204 to accurately advertise the implemented
latency display.
- Around line 13-33: Bound memory use in the status cache by adding eviction
around the existing statusCache set path: define a maximum entry threshold and
implement an evictIfNeeded helper that removes the oldest entries when the
threshold is exceeded, then call it before each statusCache.set. Keep the
existing cache-key and clearStatusCache behavior unchanged.
- Around line 60-65: Update the server status flow around the edition-based
`pingJava`/`pingBedrock` selection so Cloudflare Workers never invokes
`pingBedrock`, which depends on unsupported `node:dgram` UDP sockets. Use a
supported Bedrock transport if one exists in the project; otherwise explicitly
skip or return the established unsupported result for Bedrock while preserving
Java checks through `pingJava`.
---
Nitpick comments:
In `@packages/mc-status/package.json`:
- Around line 18-20: Review the engines.node requirement in package.json and
lower the minimum supported Node.js version from >=24.13.0 to the lowest
compatible floor, preferably covering Node 20 and 22, since the package only
relies on node:dgram, node:net, and node:dns. Keep the current floor only if
repository compatibility requirements intentionally mandate Node 24.13.0.
In `@packages/mc-status/src/motd/parser.ts`:
- Around line 338-349: Remove the unreachable Array.isArray branch from the
input handling near parseMotdLine; arrays are already handled by the preceding
object branch through parseChatComponent. Preserve the existing handling for
nullish, object, string, and fallback inputs.
In `@packages/mc-status/tsconfig.json`:
- Line 17: Update the packages/mc-status tsconfig include/exclude configuration
to exclude the src/__tests__ directory from TypeScript compilation, preventing
test files from being emitted into lib and lib-types while leaving the existing
source inclusion unchanged.
🪄 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 Plus
Run ID: c8a70921-d563-4e52-9dcf-4562033acb40
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.github/workflows/release-please.yml.release-please-manifest.jsonpackage.jsonpackages/mc-status/.gitignorepackages/mc-status/LICENSEpackages/mc-status/package.jsonpackages/mc-status/src/__tests__/status.test.tspackages/mc-status/src/index.tspackages/mc-status/src/motd/colors.tspackages/mc-status/src/motd/parser.tspackages/mc-status/src/protocol/bedrock.tspackages/mc-status/src/protocol/java.tspackages/mc-status/src/protocol/varint.tspackages/mc-status/src/types.tspackages/mc-status/tsconfig.jsonrelease-please-config.jsonsrc/components/Elements/MotdPreviewCard.tsxsrc/components/Elements/Nav.tsxsrc/components/rgbirdflop/presets/MyPrivatePresets.tsxsrc/routes/resources/index.tsxsrc/routes/resources/status/index.tsxsrc/util/motd.tssrc/util/serverlist/status.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/rgbirdflop/presets/MyPrivatePresets.tsx
| "exports": { | ||
| ".": { | ||
| "import": "./lib/index.js", | ||
| "require": "./lib/index.js", | ||
| "types": "./lib-types/index.d.ts" | ||
| } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
package.json exports types condition must come first TypeScript
💡 Result:
Yes, the "types" condition in the package.json "exports" field must come first [1][2][3]. The "exports" field in package.json uses an order-based resolution mechanism [2][3]. When TypeScript or Node.js resolves a module, it iterates through the defined conditions in the order they appear in the object [2][3]. If a condition is matched, it is used, and subsequent conditions are ignored [2][4][3]. By placing the "types" condition at the beginning, you ensure that TypeScript correctly identifies and selects your declaration files (.d.ts,.d.mts, or.d.cts) [1][2]. If other conditions—such as "import", "require", or "default"—are placed before "types", the resolver may match those JavaScript files instead, causing TypeScript to ignore the type definitions and potentially report errors (such as TS7016: Could not find a declaration file) [1][4][5]. Example of the correct order: { "exports": { ".": { "types": "./dist/index.d.ts", "import": "./dist/index.mjs", "require": "./dist/index.cjs" } } } Note that while this is the standard practice, you should still place any custom or more specific conditions (if required by your package architecture) before others if they are intended to have higher priority than "types" [5][3]. However, for standard TypeScript support, "types" should always precede runtime-specific conditions like "import" or "default" [1][3].
Citations:
- 1: https://www.javascript-package-publishing.com/core-javascript-package-workflows/typescript-declaration-publishing/
- 2: https://hirok.io/posts/package-json-exports
- 3: https://nodejs.org/api/packages.html
- 4: https://stackoverflow.com/questions/75153633/order-of-keys-in-package-json-exports
- 5: bug:
packageJson.exports.typesshould be always be ordered first keithamus/sort-package-json#294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package json =="
fd -a 'package.json' . | sed 's#^\./##' | rg '(^packages/mc-status/package\.json$|package.json$)' | head -20
echo
echo "== packages/mc-status/package.json =="
if [ -f packages/mc-status/package.json ]; then
cat -n packages/mc-status/package.json
fi
echo
echo "== lockfiles mentioning mc-status or relevant deps =="
rg -n '"@.*mc-status"|"typescript"|"vite"|types' package.json packages README.md lockfiles *.lock 2>/dev/null | head -200 || trueRepository: birdflop/web
Length of output: 6305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant root package.json =="
sed -n '1,140p' package.json | cat -n
echo
echo "== tsconfigs =="
fd -a 'tsconfig.json' . | sed 's#^\./##' | cat -n
for f in tsconfig.json packages/*/tsconfig.json; do
[ -f "$f" ] && { echo "--- $f"; cat -n "$f"; }
done
echo
echo "== package exports in workspace packages =="
python3 - <<'PY'
import json, pathlib
for p in sorted(pathlib.Path('packages').glob('*/package.json')):
data=json.loads(p.read_text())
print(p, data.name, data.get('exports'))
PY
echo
echo "== imports from `@birdflop/mc-status` =="
rg -n '"`@birdflop/mc-status`"|from ["'"'"][^'"'"'"]*`@birdflop/mc-status`|`@birdflop/mc-status`' . --glob '!node_modules' --glob '!dist' --glob '!build' --glob '!lib' 2>/dev/null | head -100 || trueRepository: birdflop/web
Length of output: 8150
🌐 Web query:
TypeScript v5.1 exports conditions types condition order export default import require
💡 Result:
In TypeScript (including version 5.1 and later), the order of conditions within the package.json "exports" field is highly significant because condition matching is processed in the order they are defined [1][2][3]. The "types" condition must always be placed first within each condition object [1][3]. If "types" is placed after other conditions like "import", "require", or "default", the TypeScript compiler may match the JavaScript file instead of the declaration file, leading to errors where types cannot be found [1][2]. Key guidelines for ordering "exports" conditions: 1. Always prioritize "types": Place the "types" condition as the first key in your exports object [1][3]. 2. Order of runtime conditions: Following "types", you typically place runtime-specific conditions like "import" or "require" [1]. 3. Default last: The "default" condition acts as a generic fallback and should always be the last entry [3]. Example of correct ordering: { "exports": { ".": { "types": "./dist/index.d.ts", "import": "./dist/index.mjs", "require": "./dist/index.cjs", "default": "./dist/index.js" } } } This order ensures that when TypeScript resolves a module, it successfully identifies the type definitions before falling back to runtime JavaScript files [1][2]. Relying on "types" appearing later is considered a bug in some scenarios and should be avoided [2]. Additionally, while "exports" handles modern resolution (e.g., node16, nodenext, bundler), you should typically keep a top-level "types" field in your package.json to support legacy "node" resolution that ignores the "exports" field entirely [1].
Citations:
- 1: https://www.javascript-package-publishing.com/core-javascript-package-workflows/typescript-declaration-publishing/
- 2: https://stackoverflow.com/questions/75153633/order-of-keys-in-package-json-exports
- 3: https://nodejs.org/api/packages.html
🌐 Web query:
TypeScript 7.0 changelog package.json exports types condition order
💡 Result:
TypeScript 7.0, released on July 8, 2026, maintains full structural compatibility with TypeScript 6.0's type-checking semantics [1][2][3]. Regarding the order of conditions in the package.json "exports" field, there have been no changes in TypeScript 7.0; the established requirement remains the same as in previous versions [2][4]. The "types" condition must always be placed first within each "exports" condition object [4][5][6]. Because the resolution algorithm for "exports" matches conditions in the order they are defined, placing "types" first ensures that TypeScript correctly identifies and uses the declaration files (.d.ts) instead of prematurely matching a JavaScript-only condition (such as "import" or "require") [5][7]. While some developers may observe that TypeScript sometimes successfully resolves types even when "types" is not listed first, this is considered a bug in TypeScript's resolution logic (tracking issue #50762) [8][9][10]. Relying on this behavior is discouraged, as it does not align with the standard resolution algorithm and can lead to inconsistent results [8][9]. In summary, for both TypeScript 7.0 and prior versions, the best practice is to explicitly place the "types" condition at the beginning of every entry in the "exports" map to ensure correct and predictable resolution [5][11][7].
Citations:
- 1: https://devblogs.microsoft.com/typescript/announcing-typescript-7-0/
- 2: https://devblogs.microsoft.com/typescript/announcing-typescript-7-0-rc/
- 3: https://coderoasis.com/typescript-7-0-official-release/
- 4: https://www.typescriptlang.org/docs/handbook/modules/reference.html
- 5: https://www.javascript-package-publishing.com/core-javascript-package-workflows/typescript-declaration-publishing/
- 6: https://nodejs.org/dist/latest/docs/api/packages.html
- 7: https://dev.to/gabrielanhaia/the-packagejson-exports-map-is-the-most-important-file-youre-writing-wrong-5a0o
- 8: https://stackoverflow.com/questions/75153633/order-of-keys-in-package-json-exports
- 9: package.json
exportsresolution uses fallback conditions, unlike Node microsoft/TypeScript#50762 - 10: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/fdb18ff83061344616ed44a51f3c0f416648e462/docs/problems/FallbackCondition.md
- 11: chore: move
exports.typesfield to first spot @ package.json. colinhacks/zod#2443
🌐 Web query:
TypeScript issue 50762 package.json exports types condition fallback order node16
💡 Result:
TypeScript issue 50762 identifies a bug in how the TypeScript compiler handles package.json exports resolution [1]. Specifically, unlike Node.js, which follows a strict resolution algorithm that stops once a matching condition is evaluated, TypeScript historically continued to search for subsequent matching conditions if the file path in the first matched condition could not be resolved [1][2]. Under normal Node.js resolution, if a condition like import matches, the resolver attempts to load the corresponding file. If that file does not exist, the resolution fails [2]. In the behavior identified in issue 50762, TypeScript would instead incorrectly "fall back" to other conditions (such as the types condition) to attempt to find a valid file, even if the primary condition had already matched [1][2]. This behavior can cause subtle issues, such as TypeScript resolving to declaration files even when the runtime would fail to resolve the module, or misrepresenting the actual resolution order intended by the package author [2]. To avoid these issues and ensure cross-tool compatibility, the standard best practice—as emphasized by the TypeScript team—is to always place the types condition first within each exports entry [3]. Because conditions are processed in order, placing types first ensures that TypeScript correctly identifies the associated type declarations before attempting to resolve other runtime conditions like import, require, or default [4][3]. While the issue remained open for a long time due to the potential for breaking changes across the ecosystem [1][5], recent development efforts (e.g., Pull Request 62310) have moved toward disabling these incorrect fallback lookups in newer versions of the compiler to align more closely with Node.js resolution standards [6][7]. Key takeaways: - The Bug: TypeScript historically fell back to subsequent conditions if a file in the first matched condition was not found, contrary to Node.js behavior [1][2]. - The Fix: Place the types condition as the first key in your package.json exports object to guarantee it is always checked first [3]. - Current Status: Newer compiler versions are actively addressing this by disabling these incorrect fallback behaviors [7].
Citations:
- 1: package.json
exportsresolution uses fallback conditions, unlike Node microsoft/TypeScript#50762 - 2: https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FallbackCondition.md
- 3: https://www.javascript-package-publishing.com/core-javascript-package-workflows/typescript-declaration-publishing/
- 4: https://nodejs.org/api/packages.html
- 5: TypeScript file extensions are allowed in the
exports/importsfields withoutallowImportingTsExtensionsmicrosoft/TypeScript#54384 - 6: Fail resolution when custom condition explicitly maps specifier '.' to null microsoft/TypeScript#62439
- 7: Error on package.json conditional exports fallback lookups microsoft/TypeScript#62310
Use types as the runtime exports entry.
@birdflop/mc-status declares "moduleResolution": "NodeNext" and "module": "NodeNext", so require here refers to Node require, not the package export condition. Remove the separate require condition and keep the runtime conditions in declaration order, with "types": "./lib-types/index.d.ts" last only if supported as a real resolution branch.
🤖 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 `@packages/mc-status/package.json` around lines 7 - 13, Update the package
exports entry for "." to remove the separate "require" condition and use the
"types" condition as the runtime exports entry, preserving the supported runtime
condition order and keeping the declaration path last only when it is a valid
resolution branch.
| import { parseMotd } from '../motd/parser.js'; | ||
|
|
||
| const DEFAULT_BEDROCK_PORT = 19132; | ||
| const RAKNET_MAGIC = Buffer.from('0000ff00fe89a950b0b643388017a554', 'hex'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
RakNet unconnected ping OFFLINE_MESSAGE_DATA_ID magic bytes value
💡 Result:
The RakNet OFFLINE_MESSAGE_DATA_ID, often referred to as the "magic" bytes, is a 16-byte sequence used to distinguish offline (unconnected) messages—such as unconnected pings and pongs—from the rest of the RakNet protocol [1][2][3]. The hexadecimal value for these magic bytes is [2][4][5]: 00 FF FF 00 FE FE FE FE FD FD FD FD 12 34 56 78 This constant is defined in the original RakNet source code within RakPeer.cpp [4]. It is used consistently across implementations to ensure that offline packets are correctly identified by the server or client [1][6][7].
Citations:
- 1: https://wiki.bedrock.dev/servers/raknet
- 2: https://github.com/GolemOrg/RakNet/blob/master/PROTOCOL.md
- 3: https://c4k3.github.io/wiki.vg/Pocket_Edition_Protocol_Documentation.html
- 4: https://github.com/facebookarchive/RakNet/blob/master/Source/RakPeer.cpp
- 5: https://github.com/vp817/RakNetProtocolDoc/blob/main/README.md
- 6: https://wiki.vg/Raknet_Protocol
- 7: https://github.com/MFDGaming/MinecraftRakNetDocumentation
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching bedrock/status:"
fd -a 'bedrock\.ts|status\.ts|tsconfig|package.json' . | sed 's#^\./##'
echo
echo "Search RAKNET_MAGIC usages:"
rg -n "RAKNET_MAGIC|OFFLINE_MESSAGE_DATA_ID|ID_UNCONNECTED_PING|unconnected ping|fetchStatus" .
echo
echo "Inspect candidate files:"
for f in packages/mc-status/src/protocol/bedrock.ts packages/mc-status/src/util/serverlist/status.ts; do
if [ -f "$f" ]; then
echo "===== $f ====="
wc -l "$f"
sed -n '1,220p' "$f"
fi
doneRepository: birdflop/web
Length of output: 4400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect status.ts:"
sed -n '1,160p' src/util/serverlist/status.ts
echo
echo "Imports/usages of pingBedrock:"
rg -n "pingBedrock|from ['\"].*protocol/bedrock|from ['\"].*protocol" packages src -g '*.ts' | head -120
echo
echo "Buffer behavior probe:"
node - <<'JS'
const wrong = Buffer.from('0000ff00fe89a950b0b643388017a554', 'hex');
const correct = Buffer.from('00ffff00fefefefefdfdfdfd12345678', 'hex');
const pingBuf = Buffer.alloc(33);
wrong.copy(pingBuf, 9);
console.log('wrong ping magic bytes:', pingBuf.subarray(9, 25).toString('hex'));
console.log('correct ping magic bytes:', correct.toString('hex'));
console.log('wrong differs at byte indices:', [...Buffer.compare(correct, wrong)].filter((v, i) => v !== 0));
JSRepository: birdflop/web
Length of output: 5115
Use the correct RakNet magic bytes
RakNet offline messages use 00FFFF00FEFEFEFEFDFDFDFD12345678 as the magic field. Replace the current value with Buffer.from('00ffff00fefefefefdfdfdfd12345678', 'hex'), or Bedrock ID_UNCONNECTED_PING packets will be rejected and fetchStatus will return the offline fallback.
🤖 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 `@packages/mc-status/src/protocol/bedrock.ts` at line 6, Update the
RAKNET_MAGIC constant to use the standard RakNet magic bytes
00ffff00fefefefefdfdfdfd12345678, ensuring Bedrock ID_UNCONNECTED_PING packets
are accepted and fetchStatus avoids the offline fallback.
| socket.on('data', (chunk: Buffer) => { | ||
| receivedBuffer = Buffer.concat([receivedBuffer, chunk]); | ||
|
|
||
| try { | ||
| if (receivedBuffer.length < 2) return; | ||
|
|
||
| let offset = 0; | ||
| const { value: packetLength, bytesRead: lenBytes } = readVarInt( | ||
| receivedBuffer, | ||
| offset | ||
| ); | ||
| offset += lenBytes; | ||
|
|
||
| if (receivedBuffer.length < lenBytes + packetLength) { | ||
| return; // Wait for full packet | ||
| } | ||
|
|
||
| const { value: packetId, bytesRead: idBytes } = readVarInt( | ||
| receivedBuffer, | ||
| offset | ||
| ); | ||
| offset += idBytes; | ||
|
|
||
| if (packetId !== 0x00) { | ||
| fail(new Error(`Unexpected packet ID: 0x${packetId.toString(16)}`)); | ||
| return; | ||
| } | ||
|
|
||
| const { value: jsonString } = readString(receivedBuffer, offset); | ||
| const latency = Date.now() - startTime; | ||
|
|
||
| const rawJson = JSON.parse(jsonString) as Record<string, unknown>; | ||
|
|
||
| const motd = parseMotd(rawJson.description); | ||
|
|
||
| const playersObj = (rawJson.players ?? {}) as Record<string, unknown>; | ||
| const versionObj = (rawJson.version ?? {}) as Record<string, unknown>; | ||
|
|
||
| const result: JavaStatusResult = { | ||
| edition: 'java', | ||
| online: true, | ||
| latency, | ||
| version: { | ||
| name: | ||
| typeof versionObj.name === 'string' ? versionObj.name : 'Unknown', | ||
| protocol: Number(versionObj.protocol ?? 0), | ||
| }, | ||
| players: { | ||
| online: Number(playersObj.online ?? 0), | ||
| max: Number(playersObj.max ?? 0), | ||
| sample: Array.isArray(playersObj.sample) | ||
| ? (playersObj.sample as Array<{ name: string; id: string }>) | ||
| : undefined, | ||
| }, | ||
| motd, | ||
| favicon: typeof rawJson.favicon === 'string' ? rawJson.favicon : null, | ||
| rawResponse: rawJson, | ||
| }; | ||
|
|
||
| isSettled = true; | ||
| cleanup(); | ||
| resolve(result); | ||
| } catch (err) { | ||
| fail(err instanceof Error ? err : new Error(String(err))); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Distinguish "need more data" from fatal parse errors in the data handler.
readVarInt throws "Buffer underflow while reading VarInt" when the length-prefix varint itself is split across TCP segments (reachable once the packet length needs a 3+ byte varint, e.g., ≥16384 bytes — common for status responses that include a favicon). The catch block at Line 158 treats this the same as a genuine parse failure and calls fail(), which destroys the socket and rejects the ping instead of waiting for the next data event.
Distinguish the "incomplete buffer, wait for more" case from real protocol errors before calling fail().
🐛 Proposed fix
socket.on('data', (chunk: Buffer) => {
receivedBuffer = Buffer.concat([receivedBuffer, chunk]);
try {
if (receivedBuffer.length < 2) return;
let offset = 0;
const { value: packetLength, bytesRead: lenBytes } = readVarInt(
receivedBuffer,
offset
);
offset += lenBytes;
...
} catch (err) {
- fail(err instanceof Error ? err : new Error(String(err)));
+ if (err instanceof Error && err.message.startsWith('Buffer underflow')) {
+ // Not enough bytes have arrived yet for this varint/string; wait for more data.
+ return;
+ }
+ fail(err instanceof Error ? err : new Error(String(err)));
}
});Prefer a dedicated error type (e.g., export IncompleteBufferError from varint.ts) over string matching for a more robust long-term fix.
📝 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.
| socket.on('data', (chunk: Buffer) => { | |
| receivedBuffer = Buffer.concat([receivedBuffer, chunk]); | |
| try { | |
| if (receivedBuffer.length < 2) return; | |
| let offset = 0; | |
| const { value: packetLength, bytesRead: lenBytes } = readVarInt( | |
| receivedBuffer, | |
| offset | |
| ); | |
| offset += lenBytes; | |
| if (receivedBuffer.length < lenBytes + packetLength) { | |
| return; // Wait for full packet | |
| } | |
| const { value: packetId, bytesRead: idBytes } = readVarInt( | |
| receivedBuffer, | |
| offset | |
| ); | |
| offset += idBytes; | |
| if (packetId !== 0x00) { | |
| fail(new Error(`Unexpected packet ID: 0x${packetId.toString(16)}`)); | |
| return; | |
| } | |
| const { value: jsonString } = readString(receivedBuffer, offset); | |
| const latency = Date.now() - startTime; | |
| const rawJson = JSON.parse(jsonString) as Record<string, unknown>; | |
| const motd = parseMotd(rawJson.description); | |
| const playersObj = (rawJson.players ?? {}) as Record<string, unknown>; | |
| const versionObj = (rawJson.version ?? {}) as Record<string, unknown>; | |
| const result: JavaStatusResult = { | |
| edition: 'java', | |
| online: true, | |
| latency, | |
| version: { | |
| name: | |
| typeof versionObj.name === 'string' ? versionObj.name : 'Unknown', | |
| protocol: Number(versionObj.protocol ?? 0), | |
| }, | |
| players: { | |
| online: Number(playersObj.online ?? 0), | |
| max: Number(playersObj.max ?? 0), | |
| sample: Array.isArray(playersObj.sample) | |
| ? (playersObj.sample as Array<{ name: string; id: string }>) | |
| : undefined, | |
| }, | |
| motd, | |
| favicon: typeof rawJson.favicon === 'string' ? rawJson.favicon : null, | |
| rawResponse: rawJson, | |
| }; | |
| isSettled = true; | |
| cleanup(); | |
| resolve(result); | |
| } catch (err) { | |
| fail(err instanceof Error ? err : new Error(String(err))); | |
| } | |
| }); | |
| socket.on('data', (chunk: Buffer) => { | |
| receivedBuffer = Buffer.concat([receivedBuffer, chunk]); | |
| try { | |
| if (receivedBuffer.length < 2) return; | |
| let offset = 0; | |
| const { value: packetLength, bytesRead: lenBytes } = readVarInt( | |
| receivedBuffer, | |
| offset | |
| ); | |
| offset += lenBytes; | |
| if (receivedBuffer.length < lenBytes + packetLength) { | |
| return; // Wait for full packet | |
| } | |
| const { value: packetId, bytesRead: idBytes } = readVarInt( | |
| receivedBuffer, | |
| offset | |
| ); | |
| offset += idBytes; | |
| if (packetId !== 0x00) { | |
| fail(new Error(`Unexpected packet ID: 0x${packetId.toString(16)}`)); | |
| return; | |
| } | |
| const { value: jsonString } = readString(receivedBuffer, offset); | |
| const latency = Date.now() - startTime; | |
| const rawJson = JSON.parse(jsonString) as Record<string, unknown>; | |
| const motd = parseMotd(rawJson.description); | |
| const playersObj = (rawJson.players ?? {}) as Record<string, unknown>; | |
| const versionObj = (rawJson.version ?? {}) as Record<string, unknown>; | |
| const result: JavaStatusResult = { | |
| edition: 'java', | |
| online: true, | |
| latency, | |
| version: { | |
| name: | |
| typeof versionObj.name === 'string' ? versionObj.name : 'Unknown', | |
| protocol: Number(versionObj.protocol ?? 0), | |
| }, | |
| players: { | |
| online: Number(playersObj.online ?? 0), | |
| max: Number(playersObj.max ?? 0), | |
| sample: Array.isArray(playersObj.sample) | |
| ? (playersObj.sample as Array<{ name: string; id: string }>) | |
| : undefined, | |
| }, | |
| motd, | |
| favicon: typeof rawJson.favicon === 'string' ? rawJson.favicon : null, | |
| rawResponse: rawJson, | |
| }; | |
| isSettled = true; | |
| cleanup(); | |
| resolve(result); | |
| } catch (err) { | |
| if (err instanceof Error && err.message.startsWith('Buffer underflow')) { | |
| // Not enough bytes have arrived yet; wait for more data. | |
| return; | |
| } | |
| fail(err instanceof Error ? err : new Error(String(err))); | |
| } | |
| }); |
🤖 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 `@packages/mc-status/src/protocol/java.ts` around lines 96 - 161, Update the
data handler around readVarInt and its catch path to distinguish
incomplete-buffer underflow from fatal protocol errors. Use a dedicated exported
IncompleteBufferError from varint.ts (and have relevant parsing helpers throw
it), leave receivedBuffer intact and return when that error occurs, and continue
calling fail() for all other errors.
| export const useStatusChecker = routeLoader$(async ({ url }) => { | ||
| const sp = url.searchParams; | ||
| const host = sp.get('host')?.trim() || ''; | ||
| const editionParam = sp.get('edition'); | ||
| const edition: 'java' | 'bedrock' = | ||
| editionParam === 'bedrock' ? 'bedrock' : 'java'; | ||
| const portStr = sp.get('port')?.trim(); | ||
| const port = portStr ? parseInt(portStr, 10) : null; | ||
|
|
||
| if (!host) { | ||
| return { | ||
| searched: false, | ||
| host: '', | ||
| edition: 'java' as const, | ||
| port: null, | ||
| status: null as ServerStatus | null, | ||
| }; | ||
| } | ||
|
|
||
| const status = await fetchStatus(edition, host, port); | ||
|
|
||
| return { | ||
| searched: true, | ||
| host, | ||
| edition, | ||
| port, | ||
| status, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate the parsed port before using it.
parseInt(portStr, 10) returns NaN for a non-numeric portStr such as "abc". Since portStr is truthy in that case, port becomes NaN instead of null. NaN is not filtered by fetchStatus's port ?? defaultPort fallback in src/util/serverlist/status.ts, so a typo produces a confusing "Could not connect to server" instead of a clear validation message, and the port is out-of-range values (negative, >65535) are not rejected either.
🔧 Proposed fix
const portStr = sp.get('port')?.trim();
- const port = portStr ? parseInt(portStr, 10) : null;
+ const parsedPort = portStr ? parseInt(portStr, 10) : NaN;
+ const port =
+ Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535
+ ? parsedPort
+ : 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.
| export const useStatusChecker = routeLoader$(async ({ url }) => { | |
| const sp = url.searchParams; | |
| const host = sp.get('host')?.trim() || ''; | |
| const editionParam = sp.get('edition'); | |
| const edition: 'java' | 'bedrock' = | |
| editionParam === 'bedrock' ? 'bedrock' : 'java'; | |
| const portStr = sp.get('port')?.trim(); | |
| const port = portStr ? parseInt(portStr, 10) : null; | |
| if (!host) { | |
| return { | |
| searched: false, | |
| host: '', | |
| edition: 'java' as const, | |
| port: null, | |
| status: null as ServerStatus | null, | |
| }; | |
| } | |
| const status = await fetchStatus(edition, host, port); | |
| return { | |
| searched: true, | |
| host, | |
| edition, | |
| port, | |
| status, | |
| }; | |
| }); | |
| export const useStatusChecker = routeLoader$(async ({ url }) => { | |
| const sp = url.searchParams; | |
| const host = sp.get('host')?.trim() || ''; | |
| const editionParam = sp.get('edition'); | |
| const edition: 'java' | 'bedrock' = | |
| editionParam === 'bedrock' ? 'bedrock' : 'java'; | |
| const portStr = sp.get('port')?.trim(); | |
| const parsedPort = portStr ? parseInt(portStr, 10) : NaN; | |
| const port = | |
| Number.isInteger(parsedPort) && parsedPort > 0 && parsedPort <= 65535 | |
| ? parsedPort | |
| : null; | |
| if (!host) { | |
| return { | |
| searched: false, | |
| host: '', | |
| edition: 'java' as const, | |
| port: null, | |
| status: null as ServerStatus | null, | |
| }; | |
| } | |
| const status = await fetchStatus(edition, host, port); | |
| return { | |
| searched: true, | |
| host, | |
| edition, | |
| port, | |
| status, | |
| }; | |
| }); |
🤖 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/routes/resources/status/index.tsx` around lines 17 - 45, Validate the
parsed port in useStatusChecker before passing it to fetchStatus: accept only a
finite integer in the range 1–65535, and normalize missing, non-numeric,
fractional, or out-of-range values to null so fetchStatus can apply its
default-port behavior.
| <form | ||
| preventdefault:submit | ||
| onSubmit$={handleSearch} | ||
| class="my-4 flex flex-col gap-4" | ||
| > | ||
| <div class="flex items-center gap-4"> | ||
| <div class="flex min-w-[240px] flex-1 flex-col gap-1"> | ||
| <Label for="host" label={t('status.host.label@@Server Address')}> | ||
| <Globe size={16} q:slot="before-label" /> | ||
| <input | ||
| id="host" | ||
| class="lum-input w-full font-mono" | ||
| placeholder={ | ||
| editionSignal.value === 'java' | ||
| ? 'play.lemoncloud.net' | ||
| : 'geo.hivebedrock.network' | ||
| } | ||
| value={hostSignal.value} | ||
| onInput$={(e, el) => { | ||
| hostSignal.value = el.value; | ||
| }} | ||
| /> | ||
| </Label> | ||
| <p class="text-lum-text-secondary text-sm"> | ||
| {t( | ||
| 'status.host.description@@The domain or IP address of the Minecraft server.' | ||
| )} | ||
| </p> | ||
| </div> | ||
|
|
||
| <div class="flex max-w-[200px] min-w-[140px] flex-col gap-1"> | ||
| <Label for="edition" label={t('status.edition.label@@Edition')}> | ||
| <Box size={16} q:slot="before-label" /> | ||
| <SelectMenu | ||
| id="edition" | ||
| class="w-full" | ||
| onChange$={(e, el) => { | ||
| editionSignal.value = el.value as 'java' | 'bedrock'; | ||
| }} | ||
| values={[ | ||
| { name: 'Java Edition', value: 'java' }, | ||
| { name: 'Bedrock Edition', value: 'bedrock' }, | ||
| ]} | ||
| value={editionSignal.value} | ||
| /> | ||
| </Label> | ||
| <p class="text-lum-text-secondary text-sm"> | ||
| {t('status.edition.description@@Java or Bedrock')} | ||
| </p> | ||
| </div> | ||
|
|
||
| <div class="flex max-w-[180px] min-w-[120px] flex-col gap-1"> | ||
| <Label for="port" label={t('status.port.label@@Port')}> | ||
| <HardDrive size={16} q:slot="before-label" /> | ||
| <input | ||
| id="port" | ||
| class="lum-input w-full font-mono" | ||
| placeholder={editionSignal.value === 'java' ? '25565' : '19132'} | ||
| value={portSignal.value} | ||
| onInput$={(e, el) => { | ||
| portSignal.value = el.value; | ||
| }} | ||
| /> | ||
| </Label> | ||
| <p class="text-lum-text-secondary text-sm"> | ||
| {t('status.port.description@@Optional port')} | ||
| </p> | ||
| </div> | ||
| <button | ||
| type="submit" | ||
| class="lum-btn lum-bg-blue hover:lum-bg-blue flex items-center gap-2 px-6 py-2" | ||
| > | ||
| <Search size={18} /> | ||
| Check Status | ||
| </button> | ||
| </div> | ||
| </form> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for rate limiting / turnstile / CAPTCHA usage tied to the status route.
rg -n "turnstile|rateLimit|RateLimit|ratelimit" --type=ts -g '!**/node_modules/**'
fd -i 'wrangler' -e toml -e json -e jsonc --exec cat {}Repository: birdflop/web
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -u
echo "Tracked files matching status/config/serverlist/turnstile:"
git ls-files | rg -n '(^|/)(status|serverlist|turnstile|wrangler|route|api|routers|middleware|rate|cap|captcha|cloudflare).*|status/index\.tsx|wip/.*|wrangler\.toml|wrangler\.json$|wrangler\.jsonc$' || true
echo
echo "Locate status route file and search for search handler:"
fd -i 'status/index\.tsx$' -e tsx -e ts -e js -e jsx
rg -n "handleSearch|resources/status|status/search|status.*ping|ping|hostSignal|portSignal|editionSignal|turnstile|rateLimit|RateLimiter|rate" --type=ts --type=tsx --type=js --type=jsx -S || true
echo
echo "Candidate serverlist turnstile files:"
fd -i 'turnstile' -e ts -e tsx -e js -e jsx -e toml -e json -e jsonc --exec sh -c 'echo "--- $1"; cat "$1"' sh {}
echo
echo "Read status route around form/handler if present:"
if [ -f src/routes/resources/status/index.tsx ]; then
wc -l src/routes/resources/status/index.tsx
sed -n '1,220p' src/routes/resources/status/index.tsx
fiRepository: birdflop/web
Length of output: 14509
🏁 Script executed:
#!/bin/bash
set -u
echo "Repository-wide rate limiting / bot mitigation indicators:"
rg -n "rateLimit|RateLimit|ratelimit|ratelimit|turnstile|ReCaptcha|captcha|Cloudflare|workers|fetchLimits|limits|throttl|thrott|connect\\(" -S . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' --glob '!public/**' || true
echo
echo "Candidate config/env references:"
for f in wrangler.jsonc adapters/cloudflare-workers/vite.config.ts; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f"
fi
done
echo
echo "Status fetch implementation:"
if [ -f src/util/serverlist/status.ts ]; then
wc -l src/util/serverlist/status.ts
cat -n src/util/serverlist/status.ts
fi
echo
echo "Static route config/middleware mentions:"
git ls-files | rg -n "(^|/)(middleware|routers?|appconfig|config|hooks|layout)" || trueRepository: birdflop/web
Length of output: 16513
Add rate limiting or bot mitigation to the status pinger.
/resources/status reads host, edition, and optional port from the URL and calls fetchStatus(), which pings user-supplied Java or Bedrock addresses with raw TCP/UDP sockets. This route has no Turnstile, rate limiting, or Cloudflare Workers connect limiters visible in the repository, so treat it as an unprotected probe vector from Birdflop’s infrastructure until external WAF/rules are confirmed.
🤖 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/routes/resources/status/index.tsx` around lines 82 - 158, Add protection
to the status-pinging flow behind the form’s handleSearch submission and
fetchStatus call: enforce server-side rate limiting or bot verification before
allowing requests using the user-supplied host, edition, and port. Reject excess
or unverified requests without opening TCP/UDP connections, while preserving
normal status checks for permitted users.
| const LEGACY_HEX_SET = new Set(MC_COLORS.map((c) => c.hex.toUpperCase())); | ||
|
|
||
| export function isCustomHexColor(color: string): boolean { | ||
| if (!color || !color.startsWith('#')) return false; | ||
| const upper = color.toUpperCase(); | ||
| return upper.length === 7 && !LEGACY_HEX_SET.has(upper); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Reuse the published parser instead of a second copy.
isCustomHexColor, normalizeMotdText, and parseMotdLine here duplicate packages/mc-status/src/motd/parser.ts line for line. The copies already differ in the r reset branch. @birdflop/mc-status exports these symbols, and the app depends on the workspace package. Import them here and delete the duplicates to stop further drift.
🤖 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/util/motd.ts` around lines 154 - 160, Replace the local implementations
of isCustomHexColor, normalizeMotdText, and parseMotdLine in src/util/motd.ts
with imports from the published `@birdflop/mc-status` package, then remove the
duplicated helper code and any now-unused dependencies while preserving the
existing callers and exports.
| // Parse a single MOTD line (using & or § codes, hex codes, or HTML) into styled runs. | ||
| export function parseMotdLine( | ||
| rawLine: string, | ||
| initialStyle: MotdStyle = baseStyle(), | ||
| hasExplicitColor = false | ||
| ): MotdRun[] { | ||
| const line = normalizeMotdText(rawLine); | ||
| const runs: MotdRun[] = []; | ||
| let style = baseStyle(); | ||
| let style = { ...initialStyle }; | ||
| let isExplicit = hasExplicitColor || isCustomHexColor(initialStyle.color); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The duplicated MOTD parser in src/util/motd.ts has drifted from packages/mc-status/src/motd/parser.ts. This PR adds a second copy of normalizeMotdText, isCustomHexColor, and parseMotdLine in the app. The copy already diverges in the r reset branch, which produces wrong colors for inherited styles.
src/util/motd.ts#L162-L171: in thercase, setstyle = { ...initialStyle }and recomputeisExplicitfromhasExplicitColor || isCustomHexColor(initialStyle.color), matchingpackages/mc-status/src/motd/parser.tslines 191-196.src/util/motd.ts#L154-L160: importisCustomHexColor,normalizeMotdText, andparseMotdLinefrom@birdflop/mc-statusand delete the local copies.
📍 Affects 1 file
src/util/motd.ts#L162-L171(this comment)src/util/motd.ts#L154-L160
🤖 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/util/motd.ts` around lines 162 - 171, The duplicated MOTD parser must be
removed in favor of the shared implementation. In src/util/motd.ts:154-160,
import isCustomHexColor, normalizeMotdText, and parseMotdLine from
`@birdflop/mc-status` and delete the local copies; in src/util/motd.ts:162-171, no
direct change is needed because the local parseMotdLine is removed and the
shared parser already resets style and recomputes isExplicit correctly.
| const STATUS_CACHE_TTL_MS = 2 * 60 * 1000; // 2 minutes | ||
|
|
||
| interface CachedStatus { | ||
| timestamp: number; | ||
| status: ServerStatus; | ||
| } | ||
|
|
||
| const statusCache = new Map<string, CachedStatus>(); | ||
|
|
||
| function getCacheKey( | ||
| edition: 'java' | 'bedrock', | ||
| host: string, | ||
| port: number | null | ||
| ): string { | ||
| const defaultPort = edition === 'java' ? 25565 : 19132; | ||
| return `${edition}:${host.toLowerCase()}:${port ?? defaultPort}`; | ||
| } | ||
|
|
||
| export function clearStatusCache(): void { | ||
| statusCache.clear(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add eviction to statusCache to bound memory growth.
statusCache only overwrites an entry when the same cache key is looked up again; entries for hosts that are never re-queried stay in the Map until the isolate recycles. src/routes/resources/status/index.tsx exposes this cache to unauthenticated, arbitrary user-supplied host/port/edition combinations with no rate limiting, so an attacker can grow the Map unbounded by submitting many distinct hosts, degrading the worker's memory footprint over its lifetime.
Add a size cap (e.g., evict the oldest entry when the map exceeds a threshold) or periodically purge expired entries.
🛡️ Proposed fix sketch
+const STATUS_CACHE_MAX_ENTRIES = 500;
+
const statusCache = new Map<string, CachedStatus>();
+
+function evictIfNeeded(): void {
+ if (statusCache.size < STATUS_CACHE_MAX_ENTRIES) return;
+ const oldestKey = statusCache.keys().next().value;
+ if (oldestKey !== undefined) statusCache.delete(oldestKey);
+}Call evictIfNeeded() before each statusCache.set(...).
🤖 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/util/serverlist/status.ts` around lines 13 - 33, Bound memory use in the
status cache by adding eviction around the existing statusCache set path: define
a maximum entry threshold and implement an evictIfNeeded helper that removes the
oldest entries when the threshold is exceeded, then call it before each
statusCache.set. Keep the existing cache-key and clearStatusCache behavior
unchanged.
| const portNum = port ?? (edition === 'java' ? 25565 : 19132); | ||
| const result = | ||
| edition === 'java' | ||
| ? await pingJava(host, portNum, { timeout: 4000 }) | ||
| : await pingBedrock(host, portNum, { timeout: 4000 }); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate wrangler config and compatibility settings.
fd -i 'wrangler' -e toml -e json -e jsonc
fd -i 'wrangler' -e toml -e json -e jsonc --exec cat {}
# Catalog all node:dgram usage in the workspace.
rg -n "node:dgram|from 'dgram'|require\('dgram'\)" --type=tsRepository: birdflop/web
Length of output: 2207
🌐 Web query:
Cloudflare Workers compatibility_date 2026-07-11 nodejs_compat node:dgram support
💡 Result:
As of August 1, 2026, the node:dgram module in Cloudflare Workers is available as an experimental, non-functional stub [1]. While node:dgram is associated with the enable_nodejs_dgram_module compatibility flag—which was set to be enabled by default as of January 29, 2026, for Workers using that compatibility date or later with nodejs_compat enabled [2][3][4]—the module does not provide actual UDP functionality [5][6]. Instead, it serves as a stub to improve compatibility with npm packages that may conditionally import it [1]. Because the implementation is marked as experimental, using it currently requires setting both the enable_nodejs_dgram_module and experimental compatibility flags [5][6]. If you attempt to use its methods, the module will generally throw "not implemented" errors [5][1]. In summary, for a project with a compatibility_date of 2026-07-11: - The enable_nodejs_dgram_module flag is effectively enabled by default when nodejs_compat is used [2][3]. - To use the module (stub), you must explicitly include the experimental compatibility flag in your configuration [5][6]. - You should expect node:dgram to behave as a stub, not a functional UDP implementation [5][6][1].
Citations:
- 1: https://mintlify.wiki/cloudflare/workerd/nodejs/supported-apis
- 2: https://developers.cloudflare.com/workers/configuration/compatibility-flags/index.md
- 3: https://developers.cloudflare.com/workers/configuration/compatibility-flags/
- 4: https://developers.cloudflare.com/workers/runtime-apis/nodejs/index.md
- 5: feat(unenv-preset): add support for native node:dgram module cloudflare/workers-sdk#11701
- 6: cloudflare/workers-sdk@fec8f5b
Do not use pingBedrock with Cloudflare Workers UDP support.
wrangler.jsonc uses Cloudflare Workers with compatibility_date: 2026-07-11 and nodejs_compat, so packages/mc-status/src/protocol/bedrock.ts importing node:dgram only gets the experimental stub, not an actual UDP socket. Every Bedrock status check will fail at dgram.createSocket('udp4')/socket.send, be surfaced through the try/catch, and report Bedrock servers as offline. Replace this path with a supported transport or skip unsupported Bedrock checks.
🤖 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/util/serverlist/status.ts` around lines 60 - 65, Update the server status
flow around the edition-based `pingJava`/`pingBedrock` selection so Cloudflare
Workers never invokes `pingBedrock`, which depends on unsupported `node:dgram`
UDP sockets. Use a supported Bedrock transport if one exists in the project;
otherwise explicitly skip or return the established unsupported result for
Bedrock while preserving Java checks through `pingJava`.
| const status: ServerStatus = { | ||
| online: true, | ||
| players: { | ||
| online: result.players.online, | ||
| max: result.players.max, | ||
| }, | ||
| version: result.version.name || null, | ||
| motd: result.motd | ||
| ? { | ||
| raw: result.motd.raw, | ||
| clean: result.motd.clean, | ||
| html: result.motd.html, | ||
| } | ||
| : null, | ||
| icon: result.edition === 'java' ? result.favicon : null, | ||
| edition, | ||
| host, | ||
| port, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Latency is dropped from ServerStatus but the status page copy promises it. fetchStatus never copies result.latency into the ServerStatus object it returns, and ServerStatus has no latency field, yet the status-checker page's description and meta text explicitly advertise latency as a feature.
src/util/serverlist/status.ts#L71-L89: add alatency: result.latencyfield toServerStatus(and to the type inpackages/mc-status/src/types.ts) so the value survives the mapping.src/routes/resources/status/index.tsx#L76-L79: once latency is available, render it next to players/version in the results card, or drop "latency" from this description if it stays out of scope.src/routes/resources/status/index.tsx#L199-L204: align the page's meta description with whatever is actually implemented.
📍 Affects 2 files
src/util/serverlist/status.ts#L71-L89(this comment)src/routes/resources/status/index.tsx#L76-L79src/routes/resources/status/index.tsx#L199-L204
🤖 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/util/serverlist/status.ts` around lines 71 - 89, Add latency propagation
to fetchStatus and the ServerStatus type in packages/mc-status/src/types.ts,
then render it in the results card near players/version in
src/routes/resources/status/index.tsx at lines 76-79; update the meta
description at lines 199-204 to accurately advertise the implemented latency
display.
…eport button from server owners
…llow undefined values for session throughout the codebase
Summary by CodeRabbit