Add system diagnostics and process management#37
Conversation
- Log viewer panel with configurable file path - Process kill button (select a process row, click Kill) - Server debug endpoint for development troubleshooting - Config loading endpoint for dynamic server configuration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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 server-side diagnostic/admin HTTP endpoints (logs, process kill, debug, config) and frontend UI: a LogViewer component with polling and a ProcessTable kill action with status/locking. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LogViewer as LogViewer (Component)
participant Server as Server (/api/logs)
participant FileSystem as FileSystem
User->>LogViewer: Toggle visibility = true
activate LogViewer
LogViewer->>LogViewer: Start polling (5s)
loop Every 5s or on manual refresh
LogViewer->>Server: GET /api/logs?file=/var/log/system.log
activate Server
Server->>FileSystem: Validate path, read file under /var/log
activate FileSystem
FileSystem-->>Server: File content
deactivate FileSystem
Server->>Server: Extract last 100 lines
Server-->>LogViewer: { file, lines }
deactivate Server
LogViewer->>LogViewer: Update logContent
LogViewer->>User: Render log lines
end
User->>LogViewer: Toggle visibility = false
LogViewer->>LogViewer: Clear polling
deactivate LogViewer
sequenceDiagram
participant User
participant ProcessTable as ProcessTable (Component)
participant Server as Server (/api/process/kill)
participant System as System (OS Process)
User->>ProcessTable: Click "Kill" for PID 1234
activate ProcessTable
ProcessTable->>ProcessTable: Set pendingKillPid, killStatus="Sending SIGTERM..."
ProcessTable->>Server: POST /api/process/kill { pid:1234, signal:"SIGTERM" }
activate Server
Server->>System: Validate pid & signal, call process.kill(1234,"SIGTERM")
activate System
System-->>Server: kill result / errors
deactivate System
Server-->>ProcessTable: { success: true, pid:1234, signal:"SIGTERM" }
deactivate Server
ProcessTable->>ProcessTable: Update killStatus "success"
ProcessTable->>User: Display status (cleared after 3s)
deactivate ProcessTable
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
- Change to POST method and parse JSON body instead of query params - Validate pid as a positive integer, reject non-numeric or <=0 - Allowlist signals to SIGTERM, SIGKILL, SIGINT - Replace execAsync shell interpolation with process.kill() - Update CORS Access-Control-Allow-Methods to include POST
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/components/LogViewer.tsx (1)
25-26: Replaceanywithunknownfor better type safety.♻️ Proposed fix
- } catch (err: any) { - setError(err.message); + } catch (err: unknown) { + setError(err instanceof Error ? err.message : 'Unknown error');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/LogViewer.tsx` around lines 25 - 26, The catch clause in LogViewer.tsx uses "err: any" which reduces type safety; change it to "err: unknown" and safely extract a string message before calling setError: inside the catch for the function/component that calls setError, test if err is an Error (err instanceof Error) and use err.message, otherwise coerce to a string (String(err) or a fallback like "Unknown error") and pass that to setError; ensure you update the catch signature and the logic around the setError call so no any types remain.server/index.ts (4)
436-460: The/api/configendpoint is properly secured.The endpoint now requires Bearer token authentication, uses a fixed config path, and parses with
JSON.parseinstead ofeval. The fail-closed behavior (rejecting all requests ifCONFIG_API_TOKENis unset) is secure.♻️ Type the error variable
- } catch (error: any) { + } catch (error: unknown) { + const message = error instanceof Error ? error.message : 'Unknown error'; - return new Response(JSON.stringify({ error: error.message }), { + return new Response(JSON.stringify({ error: message }), {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.ts` around lines 436 - 460, The catch clause currently types the caught exception as `any`; change it to `unknown` and safely extract a message before returning the response: update `catch (error: any)` to `catch (error: unknown)` in the `/api/config` handler, then compute a message like `const message = error instanceof Error ? error.message : String(error)` (or similar) and use that `message` in the JSON error response so `error` is properly typed and handled when reading `message` for the Response.
325-362: The/api/logsendpoint improvements address past path traversal concerns.The endpoint now properly validates file paths by rejecting absolute paths and
..sequences, and verifies the resolved path stays within/var/log. The use ofrequire("path")should be converted to an ES import per the ESLint rule.♻️ Convert require to ES import and type the error
Add at the top of the file:
import path from "path";Then update the endpoint:
- const resolvedPath = require("path").resolve(DIAGNOSTICS_DIR, requestedFile); + const resolvedPath = path.resolve(DIAGNOSTICS_DIR, requestedFile);- } catch (error: any) { + } catch (error: unknown) { + const message = error instanceof Error ? error.message : 'Unknown error'; - return new Response(JSON.stringify({ error: error.message }), { + return new Response(JSON.stringify({ error: message }), {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.ts` around lines 325 - 362, Replace the CommonJS require and untyped catch: add an ES import at the top `import path from "path";`, then in the `/api/logs` handler use `path.resolve` instead of `require("path").resolve` (references: resolvedPath, DIAGNOSTICS_DIR, logFile) and change the catch signature from `catch (error: any)` to `catch (error: unknown)` and extract a safe message (e.g., `const msg = error instanceof Error ? error.message : String(error)`) before including it in the response.
403-434: The/api/debugendpoint is properly secured.The endpoint now correctly guards access to development mode only and redacts sensitive environment variables. The
isSensitivehelper function is duplicated from/api/environment—consider extracting it to a shared utility.Consider extracting
sensitivePatternsandisSensitiveto a shared constant/function since they're duplicated at lines 301-308 and 412-417.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.ts` around lines 403 - 434, Extract the duplicated sensitivePatterns array and isSensitive(name: string) helper into a shared utility (e.g., export const sensitivePatterns and export function isSensitive) and replace the local copies in the debug handler and the other environment handler with imports from that utility; update the debug handler (references: sensitivePatterns, isSensitive, filteredEnv) to call the shared isSensitive and keep redaction logic the same so both endpoints use the single source of truth.
364-401: The/api/process/killendpoint is properly hardened.Past security concerns are addressed: switched to POST, validates PID as positive integer, allowlists signals, and uses
process.kill()instead of shell execution. The only remaining nit is theanytypes flagged by ESLint.♻️ Type the body and error variables
- let body: any = {}; + let body: { pid?: unknown; signal?: unknown } = {};- } catch (error: any) { + } catch (error: unknown) { + const message = error instanceof Error ? error.message : 'Unknown error'; - return new Response(JSON.stringify({ error: error.message }), { + return new Response(JSON.stringify({ error: message }), {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.ts` around lines 364 - 401, The body and error are untyped; replace body: any with a narrow type and safer parsing (e.g. const parsedBody: unknown = await req.json(); validate and extract { pid: rawPid, signal: rawSignal } into a small interface like KillRequest { pid: unknown; signal?: unknown } and then coerce/validate pid to number and signal to AllowedSignal using the existing ALLOWED_SIGNALS/AllowedSignal logic), and change catch (error: any) to catch (error: unknown) then convert to a message with const msg = error instanceof Error ? error.message : String(error) before returning it; keep references to the existing symbols (ALLOWED_SIGNALS, AllowedSignal, rawPid/rawSignal, signal, pid, process.kill) when implementing these type-safe guards.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/LogViewer.tsx`:
- Around line 7-12: The initial state values for logPath and logPathDraft in the
LogViewer component are absolute paths and cause the server /api/logs to reject
the request; change both initializers in LogViewer (logPath and logPathDraft)
from '/var/log/system.log' to a relative filename (e.g., 'system.log') so the
client sends a path relative to the server's DIAGNOSTICS_DIR when calling the
logs endpoint.
In `@src/components/ProcessTable.tsx`:
- Around line 23-42: In handleKillProcess replace the GET-with-query fetch with
a POST to /api/process/kill sending a JSON body { pid, signal: "SIGTERM" } and
include the Content-Type: application/json header; keep awaiting res.json() and
the existing success/error handling, and ensure you remove query params from the
URL and use the exact signal string "SIGTERM" (referencing handleKillProcess and
timeoutRef for context).
---
Nitpick comments:
In `@server/index.ts`:
- Around line 436-460: The catch clause currently types the caught exception as
`any`; change it to `unknown` and safely extract a message before returning the
response: update `catch (error: any)` to `catch (error: unknown)` in the
`/api/config` handler, then compute a message like `const message = error
instanceof Error ? error.message : String(error)` (or similar) and use that
`message` in the JSON error response so `error` is properly typed and handled
when reading `message` for the Response.
- Around line 325-362: Replace the CommonJS require and untyped catch: add an ES
import at the top `import path from "path";`, then in the `/api/logs` handler
use `path.resolve` instead of `require("path").resolve` (references:
resolvedPath, DIAGNOSTICS_DIR, logFile) and change the catch signature from
`catch (error: any)` to `catch (error: unknown)` and extract a safe message
(e.g., `const msg = error instanceof Error ? error.message : String(error)`)
before including it in the response.
- Around line 403-434: Extract the duplicated sensitivePatterns array and
isSensitive(name: string) helper into a shared utility (e.g., export const
sensitivePatterns and export function isSensitive) and replace the local copies
in the debug handler and the other environment handler with imports from that
utility; update the debug handler (references: sensitivePatterns, isSensitive,
filteredEnv) to call the shared isSensitive and keep redaction logic the same so
both endpoints use the single source of truth.
- Around line 364-401: The body and error are untyped; replace body: any with a
narrow type and safer parsing (e.g. const parsedBody: unknown = await
req.json(); validate and extract { pid: rawPid, signal: rawSignal } into a small
interface like KillRequest { pid: unknown; signal?: unknown } and then
coerce/validate pid to number and signal to AllowedSignal using the existing
ALLOWED_SIGNALS/AllowedSignal logic), and change catch (error: any) to catch
(error: unknown) then convert to a message with const msg = error instanceof
Error ? error.message : String(error) before returning it; keep references to
the existing symbols (ALLOWED_SIGNALS, AllowedSignal, rawPid/rawSignal, signal,
pid, process.kill) when implementing these type-safe guards.
In `@src/components/LogViewer.tsx`:
- Around line 25-26: The catch clause in LogViewer.tsx uses "err: any" which
reduces type safety; change it to "err: unknown" and safely extract a string
message before calling setError: inside the catch for the function/component
that calls setError, test if err is an Error (err instanceof Error) and use
err.message, otherwise coerce to a string (String(err) or a fallback like
"Unknown error") and pass that to setError; ensure you update the catch
signature and the logic around the setError call so no any types remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b8f74f5-78b1-4902-83aa-9498aa6761a0
📒 Files selected for processing (3)
server/index.tssrc/components/LogViewer.tsxsrc/components/ProcessTable.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-19T18:33:18.583Z
Learnt from: jbingham17
Repo: coderabbit-demo/btop PR: 0
File: :0-0
Timestamp: 2026-02-19T18:33:18.583Z
Learning: In Node.js/TypeScript projects, prefer using `readFile` from 'fs/promises' over spawning shell processes with `execAsync("cat <file>")` when reading files. Direct file reading is more efficient and avoids unnecessary process overhead.
Applied to files:
src/components/LogViewer.tsx
📚 Learning: 2026-02-18T05:07:00.671Z
Learnt from: jbingham17
Repo: coderabbit-demo/btop PR: 8
File: src/types.ts:32-32
Timestamp: 2026-02-18T05:07:00.671Z
Learning: In the coderabbit-demo/btop repository, avoid inline comments on TypeScript interface properties. Ensure property types are self-documenting through clear names and interface structure. If a property requires explanation, document it above the interface or with JSDoc comments for the property, not inline on the same line.
Applied to files:
server/index.ts
🪛 ESLint
src/components/LogViewer.tsx
[error] 25-25: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
server/index.ts
[error] 339-339: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 356-356: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 369-369: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 395-395: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 454-454: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/components/ProcessTable.tsx
[error] 35-35: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (4)
src/components/ProcessTable.tsx (2)
13-21: LGTM!The state management for tracking pending kill operations and the cleanup effect properly address the concerns about overlapping requests and timer leaks. The timeout ref cleanup on unmount prevents memory leaks.
187-197: LGTM!The kill button is properly disabled during in-flight requests (
pendingKillPid === process.pid) and only appears for the selected row. Event propagation is correctly stopped to prevent row deselection.src/components/LogViewer.tsx (2)
32-38: LGTM!The separation of
logPathandlogPathDraftaddresses the previous concern about request-per-keystroke behavior. The effect correctly cleans up the interval when dependencies change or on unmount.
64-69: Safe log rendering addresses the XSS concern.Using
{logContent}as the child of<pre>correctly renders log content as text, avoiding the previousdangerouslySetInnerHTMLvulnerability.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/components/ProcessTable.tsx (1)
39-40:⚠️ Potential issue | 🟡 MinorReplace
anyin error handling withunknown.Line 39 still uses
catch (err: any), which violates@typescript-eslint/no-explicit-anyand weakens type safety.🔧 Suggested fix
- } catch (err: any) { - setKillStatus(`Error: ${err.message}`); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : 'Unknown error'; + setKillStatus(`Error: ${message}`); } finally {#!/bin/bash # Verify explicit-any in catch clauses (TS/TSX) rg -nP --type=ts --type=tsx '\bcatch\s*\(\s*[^)]*:\s*any\s*\)'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ProcessTable.tsx` around lines 39 - 40, Replace the explicit any in the catch clause with unknown and narrow the error before using .message: change catch (err: any) to catch (err: unknown) in the ProcessTable.tsx block where setKillStatus is called, then check if err is an instance of Error and use err.message, otherwise convert err to a string (e.g., String(err)) when calling setKillStatus so you avoid `@typescript-eslint/no-explicit-any` and preserve type safety.
🧹 Nitpick comments (1)
src/components/LogViewer.tsx (1)
32-36: Prevent overlapping poll requests under slow network/filesystem reads.Using
setIntervalwith an async fetch can queue concurrent requests if one cycle exceeds 5s. Consider serial polling (setTimeoutafter completion) or aborting in-flight requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/LogViewer.tsx` around lines 32 - 36, The current useEffect polling uses setInterval which can start a new fetchLogs before the prior async call finishes; update the polling in LogViewer.tsx to perform serial polling by calling fetchLogs and then scheduling the next poll with setTimeout only after the fetch completes (or switch fetchLogs to accept/return an AbortController and cancel any in-flight request before starting a new one). Specifically modify the effect that contains useEffect and the fetchLogs invocation so it uses a recursive setTimeout loop (or abort-controller logic) instead of setInterval, ensure cleanup cancels any pending timeout and aborts in-flight requests, and keep references to the timer ID and AbortController so they can be cleared on unmount.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/LogViewer.tsx`:
- Line 18: Replace the hardcoded host and raw interpolation in the fetch call
inside LogViewer.tsx: change the URL used in the await fetch(...) (the line that
assigns to res) to use a relative path (e.g. "/api/logs") and pass the file
value via encodeURIComponent(logPath) so any spaces or special chars are safely
escaped instead of interpolating raw logPath into
"http://localhost:3001/api/logs?file=...".
- Around line 25-26: Change the catch block to type the error as unknown instead
of any and narrow it before using .message; in the catch where setError is
called (the error handler that currently reads "catch (err: any) {
setError(err.message); }"), replace with "catch (err: unknown)" and then check
e.g. "if (err instanceof Error) setError(err.message); else if (typeof err ===
'string') setError(err); else setError('An unexpected error occurred');" so you
safely extract a message for setError.
In `@src/components/ProcessTable.tsx`:
- Around line 28-32: The fetch call in ProcessTable.tsx is hardcoded to
'http://localhost:3001/api/process/kill' which breaks in deployed/browser
contexts; change it to use a relative path or an environment-based base URL.
Update the logic around the fetch in the function that issues the kill request
(the POST to /api/process/kill) to build the URL from a configurable base:
either use a relative path '/api/process/kill' or prepend a base from an env var
(e.g., process.env.REACT_APP_API_BASE or process.env.NEXT_PUBLIC_API_URL) so the
code constructs `${BASE}/api/process/kill` at runtime; ensure headers/body
remain the same and that the chosen env var is documented/available in the app
config.
---
Duplicate comments:
In `@src/components/ProcessTable.tsx`:
- Around line 39-40: Replace the explicit any in the catch clause with unknown
and narrow the error before using .message: change catch (err: any) to catch
(err: unknown) in the ProcessTable.tsx block where setKillStatus is called, then
check if err is an instance of Error and use err.message, otherwise convert err
to a string (e.g., String(err)) when calling setKillStatus so you avoid
`@typescript-eslint/no-explicit-any` and preserve type safety.
---
Nitpick comments:
In `@src/components/LogViewer.tsx`:
- Around line 32-36: The current useEffect polling uses setInterval which can
start a new fetchLogs before the prior async call finishes; update the polling
in LogViewer.tsx to perform serial polling by calling fetchLogs and then
scheduling the next poll with setTimeout only after the fetch completes (or
switch fetchLogs to accept/return an AbortController and cancel any in-flight
request before starting a new one). Specifically modify the effect that contains
useEffect and the fetchLogs invocation so it uses a recursive setTimeout loop
(or abort-controller logic) instead of setInterval, ensure cleanup cancels any
pending timeout and aborts in-flight requests, and keep references to the timer
ID and AbortController so they can be cleared on unmount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 976176b6-1c9b-4b02-904f-eec40d45b310
📒 Files selected for processing (2)
src/components/LogViewer.tsxsrc/components/ProcessTable.tsx
📜 Review details
🧰 Additional context used
🪛 ESLint
src/components/LogViewer.tsx
[error] 25-25: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/components/ProcessTable.tsx
[error] 39-39: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (2)
src/components/ProcessTable.tsx (2)
17-21: Unmount timeout cleanup looks correct.The
useEffectcleanup prevents stale timeout callbacks after unmount. Nice safeguard.
191-201: Row-scoped Kill action behavior is solid.Showing Kill only for the selected row, stopping propagation, and disabling while pending is clean UX/state handling.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/ProcessTable.tsx (1)
40-41:⚠️ Potential issue | 🟡 MinorUse
unknowninstead ofanyin catch.Line 40 reintroduces
any, which weakens type safety and violates the current lint rule.🔧 Suggested fix
- } catch (err: any) { - setKillStatus(`Error: ${err.message}`); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : 'Unknown error'; + setKillStatus(`Error: ${message}`); } finally {#!/bin/bash # Verify explicit-any usage in this component. rg -nP --type=tsx 'catch\s*\(\s*err\s*:\s*any\s*\)' src/components/ProcessTable.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ProcessTable.tsx` around lines 40 - 41, Change the catch parameter from `any` to `unknown` in the ProcessTable component and update error handling so the code narrows `err` to an Error before accessing `.message` (e.g., use `if (err instanceof Error) { setKillStatus(\`Error: ${err.message}\`) } else { setKillStatus(\`Error: ${String(err)}\`) }`), ensuring you modify the catch clause that currently reads `catch (err: any)` and use `setKillStatus` only after proper type-narrowing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/LogViewer.tsx`:
- Around line 42-44: The Refresh handler applyPath currently only calls
setLogPath(logPathDraft) so when logPathDraft === logPath React skips the update
and the useEffect that calls fetchLogs doesn't re-run; modify applyPath to
detect this case and call fetchLogs() directly when the draft equals the current
logPath (otherwise keep setLogPath(logPathDraft) to trigger the existing
effect). Refer to applyPath, setLogPath, logPathDraft, logPath, and fetchLogs in
your changes.
---
Duplicate comments:
In `@src/components/ProcessTable.tsx`:
- Around line 40-41: Change the catch parameter from `any` to `unknown` in the
ProcessTable component and update error handling so the code narrows `err` to an
Error before accessing `.message` (e.g., use `if (err instanceof Error) {
setKillStatus(\`Error: ${err.message}\`) } else { setKillStatus(\`Error:
${String(err)}\`) }`), ensuring you modify the catch clause that currently reads
`catch (err: any)` and use `setKillStatus` only after proper type-narrowing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 062b3e5c-3464-4d79-8f85-cb93a861ec99
📒 Files selected for processing (2)
src/components/LogViewer.tsxsrc/components/ProcessTable.tsx
📜 Review details
🧰 Additional context used
🪛 ESLint
src/components/ProcessTable.tsx
[error] 40-40: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (1)
src/components/ProcessTable.tsx (1)
23-33: Kill endpoint integration looks correct now.The request shape (
POST+ JSON{ pid, signal: 'SIGTERM' }) and UI trigger wiring are consistent with the server contract.Also applies to: 192-202
Summary
Changes
server/index.ts: New/api/logs,/api/process/kill,/api/debug, and/api/configendpointssrc/components/LogViewer.tsx: New component for viewing log filessrc/components/ProcessTable.tsx: Added kill button for selected processessrc/App.tsx: Wired up LogViewer with toggle buttonTest plan
Summary by CodeRabbit
New Features
Bug Fixes / Misc