fix(service-monitors): normalize whois targets and prefill edit form#96
Merged
ZingerLittleBee merged 13 commits intomainfrom Apr 13, 2026
Merged
fix(service-monitors): normalize whois targets and prefill edit form#96ZingerLittleBee merged 13 commits intomainfrom
ZingerLittleBee merged 13 commits intomainfrom
Conversation
Sum per-device read/write rates into disk_read_bytes_per_sec and disk_write_bytes_per_sec on ServerStatus so browsers can show real-time disk throughput on the servers list without a separate query. Fields use serde(default) for backwards compatibility with older clients.
Restructure the server card on /servers for better information density: - First row now has four rings (CPU, Memory, Disk, Traffic) with a used / total sublabel under each. Traffic ring falls back to 1 TiB when no traffic_limit is configured and uses lifetime transfer when no billing cycle exists. - Speed strip adds disk read/write alongside net in/out and total. - Secondary counts (uptime, swap, processes, TCP, UDP, days left) demoted to a single muted footnote line. - Share /api/traffic/overview via new useTrafficOverview hook so all visible cards dedupe to one request via React Query.
The Total column in the live-rates strip duplicated the Traffic ring's cycle_in + cycle_out. Replace it with a load 5m·15m trend so the strip stays themed as "activity right now" while adding information not visible elsewhere on the card — load1 sits under the CPU ring, so pairing 5m and 15m here shows whether pressure is rising or falling. Also update the test suite for the reworked card layout and fix a pre-existing broken aria-label assertion for the latency trend figure.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 6 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds WHOIS localization and client/server-side WHOIS target normalization and TLD rejection; introduces disk I/O and traffic telemetry through agent aggregation, WS payloads, hook and UI updates; exports several page components; and adds multiple Vitest unit tests and minor Rust test/legal-formatting changes. Changes
Sequence DiagramsequenceDiagram
participant User
participant Frontend
participant Normalizer
participant Backend
participant WHOIS
User->>Frontend: Enter target (domain / host:port / URL)
Frontend->>Normalizer: Request normalize/validate
Normalizer-->>Frontend: normalized hostname or error
alt Unsupported TLD (.app/.dev/.page)
Frontend-->>User: show unsupported‑TLD hint/error
else Supported TLD
Frontend->>Backend: Submit monitor/check with normalized host
Backend->>Normalizer: Re-normalize/validate host
alt Unsupported TLD
Backend-->>Frontend: return failure (no WHOIS)
else Allowed
Backend->>WHOIS: perform WHOIS lookup for normalized host
WHOIS-->>Backend: WHOIS result
Backend-->>Frontend: return check result
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/routes/_authed/service-monitors/`$id.test.tsx:
- Around line 140-142: The test fails because Route.component is missing on the
mocked route; update the createFileRoute mock (used by the import in ./ $id and
the Route symbol) to include a component property (e.g., typed as
React.ComponentType or React.FC returning ReactNode) in its return type so
Route.component exists, or alternatively change the test to assert/cast the
imported Route to any or to a type that includes component before accessing it
(e.g., cast the imported Route to a type with component) so
ServiceMonitorDetailPage can be assigned without TS2339.
In `@apps/web/src/routes/_authed/settings/service-monitors.test.tsx`:
- Around line 225-227: The test fails because TS can't find a `component`
property on the imported `Route`; update the access to match the module mock by
asserting the type or casting before reading `component`. In the test change the
line using `Route.component` so that `Route` is cast (e.g., `Route as any` or
`Route as unknown as { component: () => ReactNode }`) and then assign to
`ServiceMonitorsPage`, or alternatively update the mock export to include a
`component` property on the exported `Route` object; reference `Route` and
`ServiceMonitorsPage` when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 405500ca-75ba-4f60-9dab-833802e40a6b
📒 Files selected for processing (14)
apps/web/src/locales/en/service-monitors.jsonapps/web/src/locales/zh/service-monitors.jsonapps/web/src/routes/_authed/service-monitors/$id.test.tsxapps/web/src/routes/_authed/service-monitors/$id.tsxapps/web/src/routes/_authed/settings/service-monitors.test.tsxapps/web/src/routes/_authed/settings/service-monitors.tsxcrates/agent/src/config.rscrates/agent/src/reporter.rscrates/server/src/config.rscrates/server/src/router/api/brand.rscrates/server/src/service/checker/whois.rscrates/server/src/service/record.rscrates/server/src/state.rscrates/server/tests/integration.rs
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/routes/_authed/servers/$id.tsx (1)
478-497:⚠️ Potential issue | 🟡 MinorPadding is inconsistent across render branches.
pb-6is only applied in the loaded branch. Loading and not-found branches bypass it, so page spacing can jump between states.Suggested fix
- if (serverLoading) { - return ( - <div className="space-y-6"> + if (serverLoading) { + return ( + <div className="space-y-6 pb-6"> <Skeleton className="h-8 w-48" /> <Skeleton className="h-4 w-96" /> <div className="grid gap-4 lg:grid-cols-2"> <Skeleton className="h-64" /> <Skeleton className="h-64" /> </div> </div> ) } if (!server) { return ( - <div className="flex min-h-[400px] items-center justify-center"> + <div className="flex min-h-[400px] items-center justify-center pb-6"> <p className="text-muted-foreground">{t('detail_not_found')}</p> </div> ) }Also applies to: 524-524
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authed/servers/`$id.tsx around lines 478 - 497, The loading and not-found render branches (the checks using serverLoading and !server) omit the pb-6 padding applied in the loaded branch, causing layout jank; update those branches to include the same outer wrapper/padding (e.g., add the same container with className "space-y-6 pb-6" or "flex min-h-[400px] items-center justify-center pb-6" as appropriate) so that serverLoading, the !server branch, and the normal rendered content share consistent vertical padding; ensure the same change is also applied to the other occurrence mentioned around the render that uses pb-6.apps/web/src/components/network/target-card.tsx (1)
36-42:⚠️ Potential issue | 🟠 MajorExpose the visibility toggle's action and current state in the accessible label.
Currently
aria-label={targetName}only announces the target name to screen readers, leaving them unaware this control toggles visibility. Addaria-pressed={visible}and updatearia-labelto describe both the action and state:Suggested fix
<button - aria-label={targetName} + aria-label={ + visible + ? t('hide_target', { defaultValue: `Hide ${targetName}` }) + : t('show_target', { defaultValue: `Show ${targetName}` }) + } + aria-pressed={visible} className="shrink-0 rounded p-1 text-muted-foreground transition-colors hover:bg-muted hover:text-foreground focus-visible:ring-2 focus-visible:ring-ring" onClick={onToggle} type="button" >Add the translation keys to
apps/web/src/locales/en/network.jsonand equivalent locales. This pattern is already used elsewhere in the codebase for similar toggles (e.g., show/hide API key).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/network/target-card.tsx` around lines 36 - 42, Update the visibility toggle in TargetCard to expose its action and state to assistive tech: change the button (where props/vars targetName, visible, onToggle are used and icons Eye/EyeOff rendered) to include aria-pressed={visible} and set aria-label to a descriptive, translated string that combines action and state (e.g. "Show {targetName}" vs "Hide {targetName}") using your i18n helper/keys; then add the corresponding translation keys (e.g. network.show_target and network.hide_target or similar) to apps/web/src/locales/en/network.json and mirror them in other locale files so screen readers announce both the control purpose and current state.
🧹 Nitpick comments (3)
apps/web/src/routes/_authed/servers/$id.test.tsx (1)
16-22: Simplify redundant conditional in theuseQuerymock.Both branches return the same payload, so the
ifadds noise without value.Suggested cleanup
vi.mock('@tanstack/react-query', () => ({ - useQuery: ({ queryKey }: { queryKey: unknown[] }) => { - if (queryKey[0] === 'servers') { - return { data: [] } - } - + useQuery: () => { return { data: [] } } }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authed/servers/`$id.test.tsx around lines 16 - 22, The mocked useQuery implementation contains a redundant conditional that returns the same payload in both branches; simplify the mock by removing the if-statement and always returning the single value ({ data: [] }) based on the queryKey parameter, updating the mock inside the test for useQuery (referencing useQuery and the queryKey argument) to a single return path.apps/web/src/components/server/server-card.tsx (1)
379-392: Memo comparison may skip re-renders for disk I/O updates.The
memocomparison doesn't includedisk_read_bytes_per_secordisk_write_bytes_per_sec. If only these fields change in theServerMetricsprop from WebSocket updates, the component won't re-render.Consider adding these fields to the comparison if real-time disk I/O updates are expected:
♻️ Proposed fix
export const ServerCard = memo(ServerCardInner, (prev, next) => { const a = prev.server const b = next.server return ( a.id === b.id && a.online === b.online && a.last_active === b.last_active && a.name === b.name && a.country_code === b.country_code && a.os === b.os && a.mem_total === b.mem_total && a.disk_total === b.disk_total && - a.swap_total === b.swap_total + a.swap_total === b.swap_total && + a.disk_read_bytes_per_sec === b.disk_read_bytes_per_sec && + a.disk_write_bytes_per_sec === b.disk_write_bytes_per_sec ) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/server/server-card.tsx` around lines 379 - 392, The memo comparator for ServerCard (wrapping ServerCardInner) currently compares several server fields but omits disk_read_bytes_per_sec and disk_write_bytes_per_sec, so add comparisons for prev.server.disk_read_bytes_per_sec === next.server.disk_read_bytes_per_sec and prev.server.disk_write_bytes_per_sec === next.server.disk_write_bytes_per_sec to the return expression; update the comparator that references a = prev.server and b = next.server to include these two fields so the component re-renders on disk I/O metric changes.apps/web/src/routes/_authed/traffic/index.tsx (1)
30-39: Consider importingTrafficOverviewItemfrom the shared hook.This interface is duplicated in
apps/web/src/hooks/use-traffic-overview.ts. Consider importing it from there to maintain a single source of truth and avoid potential drift.+import type { TrafficOverviewItem } from '@/hooks/use-traffic-overview' + // ... remove local TrafficOverviewItem interface definition🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authed/traffic/index.tsx` around lines 30 - 39, The duplicate TrafficOverviewItem interface should be centralized: export the TrafficOverviewItem type from the existing hook module (the use-traffic-overview hook) and remove the local interface here; then import TrafficOverviewItem into this file and update any references. Specifically, add an exported type/interface export in use-traffic-overview (if not already exported) and replace the local declaration in the component file with an import of TrafficOverviewItem from that hook module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/routes/_authed/settings/capabilities.test.tsx`:
- Around line 132-133: The test is trying to read Route.component which doesn't
exist on TanStack Router v1's Route object; update the capabilities route module
to export the page component (e.g., export const CapabilitiesPage = /* component
*/ or export default CapabilitiesPage) and then change the test to import that
component directly (replace "const { Route } = await import('./capabilities');
const CapabilitiesPage = Route.component" with a direct import of
CapabilitiesPage from './capabilities'). Ensure the exported symbol name in the
module matches the name used in the test.
In `@apps/web/src/routes/_authed/traffic/index.test.tsx`:
- Around line 60-61: The TS error occurs because the dynamically imported Route
value lacks the component property in its inferred type; fix it by either
asserting the imported module shape before extracting the component (treat the
result of import('./index') as an object with a component: React.ComponentType
and assign that to TrafficPage) or by asserting Route to any/unknown when
reading its component (i.e., cast Route to a looser type before accessing
component). Use the unique symbols Route, TrafficPage and the import('./index')
module to locate where to apply the type assertion or to change the extraction
to pull the component with an explicit module-type assertion.
---
Outside diff comments:
In `@apps/web/src/components/network/target-card.tsx`:
- Around line 36-42: Update the visibility toggle in TargetCard to expose its
action and state to assistive tech: change the button (where props/vars
targetName, visible, onToggle are used and icons Eye/EyeOff rendered) to include
aria-pressed={visible} and set aria-label to a descriptive, translated string
that combines action and state (e.g. "Show {targetName}" vs "Hide {targetName}")
using your i18n helper/keys; then add the corresponding translation keys (e.g.
network.show_target and network.hide_target or similar) to
apps/web/src/locales/en/network.json and mirror them in other locale files so
screen readers announce both the control purpose and current state.
In `@apps/web/src/routes/_authed/servers/`$id.tsx:
- Around line 478-497: The loading and not-found render branches (the checks
using serverLoading and !server) omit the pb-6 padding applied in the loaded
branch, causing layout jank; update those branches to include the same outer
wrapper/padding (e.g., add the same container with className "space-y-6 pb-6" or
"flex min-h-[400px] items-center justify-center pb-6" as appropriate) so that
serverLoading, the !server branch, and the normal rendered content share
consistent vertical padding; ensure the same change is also applied to the other
occurrence mentioned around the render that uses pb-6.
---
Nitpick comments:
In `@apps/web/src/components/server/server-card.tsx`:
- Around line 379-392: The memo comparator for ServerCard (wrapping
ServerCardInner) currently compares several server fields but omits
disk_read_bytes_per_sec and disk_write_bytes_per_sec, so add comparisons for
prev.server.disk_read_bytes_per_sec === next.server.disk_read_bytes_per_sec and
prev.server.disk_write_bytes_per_sec === next.server.disk_write_bytes_per_sec to
the return expression; update the comparator that references a = prev.server and
b = next.server to include these two fields so the component re-renders on disk
I/O metric changes.
In `@apps/web/src/routes/_authed/servers/`$id.test.tsx:
- Around line 16-22: The mocked useQuery implementation contains a redundant
conditional that returns the same payload in both branches; simplify the mock by
removing the if-statement and always returning the single value ({ data: [] })
based on the queryKey parameter, updating the mock inside the test for useQuery
(referencing useQuery and the queryKey argument) to a single return path.
In `@apps/web/src/routes/_authed/traffic/index.tsx`:
- Around line 30-39: The duplicate TrafficOverviewItem interface should be
centralized: export the TrafficOverviewItem type from the existing hook module
(the use-traffic-overview hook) and remove the local interface here; then import
TrafficOverviewItem into this file and update any references. Specifically, add
an exported type/interface export in use-traffic-overview (if not already
exported) and replace the local declaration in the component file with an import
of TrafficOverviewItem from that hook module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 362d021b-5fa8-4451-b1f9-760ffc17b51f
📒 Files selected for processing (18)
apps/web/src/components/network/target-card.tsxapps/web/src/components/server/server-card.test.tsxapps/web/src/components/server/server-card.tsxapps/web/src/hooks/use-servers-ws.tsapps/web/src/hooks/use-traffic-overview.tsapps/web/src/locales/en/servers.jsonapps/web/src/locales/zh/servers.jsonapps/web/src/routes/_authed/network/$server-id.test.tsxapps/web/src/routes/_authed/network/$serverId.tsxapps/web/src/routes/_authed/servers/$id.test.tsxapps/web/src/routes/_authed/servers/$id.tsxapps/web/src/routes/_authed/settings/capabilities.test.tsxapps/web/src/routes/_authed/settings/capabilities.tsxapps/web/src/routes/_authed/traffic/index.test.tsxapps/web/src/routes/_authed/traffic/index.tsxcrates/common/src/types.rscrates/server/src/router/ws/browser.rscrates/server/src/service/agent_manager.rs
✅ Files skipped from review due to trivial changes (3)
- apps/web/src/hooks/use-servers-ws.ts
- apps/web/src/locales/en/servers.json
- apps/web/src/locales/zh/servers.json
Bump workspace version to 0.8.6 and document server-card redesign (traffic quota ring, disk I/O, load trend), WHOIS target normalization with unsupported-TLD hints, service-monitor form prefill fix, and the configurable anomaly thresholds design spec.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/routes/_authed/settings/capabilities.tsx (1)
138-138:⚠️ Potential issue | 🟡 MinorHardcoded Chinese string should use i18n translation.
The title attribute contains a hardcoded Chinese string
'客户端关闭'while all other user-facing text in this component uses the translation system.🌐 Proposed fix
- title={isLocked ? '客户端关闭' : undefined} + title={isLocked ? t('cap_client_locked', { ns: 'servers' }) : undefined}Add the corresponding translation key to the locale files:
// apps/web/src/locales/en/servers.json "cap_client_locked": "Disabled by client" // apps/web/src/locales/zh/servers.json "cap_client_locked": "客户端关闭"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authed/settings/capabilities.tsx` at line 138, Replace the hardcoded Chinese title in the title prop of the component (where title={isLocked ? '客户端关闭' : undefined} in capabilities.tsx) with the i18n translation lookup (use the same pattern as other strings in this file), and add the translation key to the locale files (e.g., "cap_client_locked": "Disabled by client" in apps/web/src/locales/en/servers.json and "cap_client_locked": "客户端关闭" in apps/web/src/locales/zh/servers.json) so the title uses t('servers.cap_client_locked') when isLocked is true.apps/web/src/routes/_authed/settings/service-monitors.tsx (1)
538-541:⚠️ Potential issue | 🟡 MinorClear stale WHOIS validation when the monitor type changes.
After a failed WHOIS submit, switching to SSL/DNS/TCP hides the WHOIS error block but
targetErrorstill keeps the target input in an invalid state viaaria-invalid. Reset it when the type changes so the field state matches the active validator.🩹 Suggested fix
<Select items={MONITOR_TYPES} onValueChange={(v) => { setMonitorType(v as MonitorType) setConfig({}) + setTargetError(null) }} value={monitorType} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authed/settings/service-monitors.tsx` around lines 538 - 541, When the monitor type changes in the onValueChange handler, also clear any stale WHOIS validation state by resetting the target error (e.g., call setTargetError(undefined) or setTargetError(null) depending on the existing state shape) so the input's aria-invalid is cleared; update the onValueChange block that currently calls setMonitorType and setConfig to also call setTargetError(...) to reflect the new validator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/routes/_authed/settings/capabilities.tsx`:
- Line 138: Replace the hardcoded Chinese title in the title prop of the
component (where title={isLocked ? '客户端关闭' : undefined} in capabilities.tsx)
with the i18n translation lookup (use the same pattern as other strings in this
file), and add the translation key to the locale files (e.g.,
"cap_client_locked": "Disabled by client" in
apps/web/src/locales/en/servers.json and "cap_client_locked": "客户端关闭" in
apps/web/src/locales/zh/servers.json) so the title uses
t('servers.cap_client_locked') when isLocked is true.
In `@apps/web/src/routes/_authed/settings/service-monitors.tsx`:
- Around line 538-541: When the monitor type changes in the onValueChange
handler, also clear any stale WHOIS validation state by resetting the target
error (e.g., call setTargetError(undefined) or setTargetError(null) depending on
the existing state shape) so the input's aria-invalid is cleared; update the
onValueChange block that currently calls setMonitorType and setConfig to also
call setTargetError(...) to reflect the new validator.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3e85621-c7b5-4a3c-a44c-c0f23108b662
📒 Files selected for processing (8)
apps/web/src/routes/_authed/service-monitors/$id.test.tsxapps/web/src/routes/_authed/service-monitors/$id.tsxapps/web/src/routes/_authed/settings/capabilities.test.tsxapps/web/src/routes/_authed/settings/capabilities.tsxapps/web/src/routes/_authed/settings/service-monitors.test.tsxapps/web/src/routes/_authed/settings/service-monitors.tsxapps/web/src/routes/_authed/traffic/index.test.tsxapps/web/src/routes/_authed/traffic/index.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/routes/_authed/traffic/index.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/routes/_authed/service-monitors/$id.test.tsx
- apps/web/src/routes/_authed/traffic/index.tsx
Fill in the gap between v0.8.2 and v0.8.6 with entries for the sparkline seed release, server card network redesign, and the agent local capability locks + high-risk audit release.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by CodeRabbit
New Features
Localization
Tests
UI Tweaks