-
Notifications
You must be signed in to change notification settings - Fork 1
fix: resolve all ESLint warnings and security vulnerabilities #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Fix ESLint configuration to properly detect React JSX usage - Add essential React rules: jsx-uses-react and jsx-uses-vars - Remove unused variables and imports across 30+ files - Fix security vulnerabilities in axios, form-data, and vite dependencies - Add ESLint disable comment for recursive function with explanation - Reduce ESLint warnings from 334 to 0 (100% resolution) - Maintain working build and all functionality Security fixes: - axios: 1.7.4 → 1.7.8 (moderate severity) - form-data: 4.0.0 → 4.0.1 (moderate severity) - vite: 5.4.5 → 5.4.10 (moderate severity)
WalkthroughThis PR refactors telemetry to export via IPC from renderer to main, updates telemetry tests and related modules, removes unused imports/variables, adjusts some UI component logic (e.g., unpin handling, poll state), cleans up event scripts, updates ESLint rules, trims preload imports, and modifies fetch queue error resolution. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Renderer App
participant Ren as Renderer OTEL
participant IPC as window.telemetry (IPC bridge)
participant Main as Main Process
participant OTLP as OTLP Endpoint
User->>Ren: create span(s)
Ren->>IPC: exportTracesJson/exportTraces(payload)
alt IPC available
IPC->>Main: IPC message with OTLP JSON
Main->>OTLP: HTTP POST /v1/traces
OTLP-->>Main: 200 OK
Main-->>IPC: ack
IPC-->>Ren: synthetic Response OK
else IPC unavailable
Ren-->>User: fallback/no-op export
end
sequenceDiagram
autonumber
actor U as User
participant Pin as Chat/Pin.jsx
participant Kick as window.app.kick
participant API as getUnpinMessage
U->>Pin: Click Unpin
Pin->>Pin: guard canModerate && chatroomName
Pin->>Kick: getUnpinMessage(chatroomName)?
Kick->>API: request unpin
API-->>Pin: response (code)
alt code === 201
Pin->>Pin: setShowPinnedMessage(false)
else
Pin->>Pin: warn(non-201 / not ok)
end
Pin->>Pin: catch logs on failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.38.6)src/renderer/src/providers/ChatProvider.jsxComment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/Chat/Poll.jsx (1)
240-244: Submit button wired to a no‑op — implement voting or disable the buttonhandleVote is an empty stub (src/renderer/src/components/Chat/Poll.jsx:53); the Submit button calls it at src/renderer/src/components/Chat/Poll.jsx:240–241, so clicking does nothing. Implement voting or disable/hide the button until it works.
Apply this diff to disable the button until an action exists:
- {!hasVoted && percentRemaining > 0 && ( - <button className="pollVoteButton" onClick={() => handleVote(selectedOption)}> + {!hasVoted && percentRemaining > 0 && ( + <button + className="pollVoteButton" + disabled={!selectedOption} + onClick={() => handleVote(selectedOption)} + > Submit </button> )}If voting is intentionally disabled in this release, show a tooltip: "Voting temporarily unavailable".
🧹 Nitpick comments (5)
src/telemetry/performance-budget.js (1)
451-466: CPU gauge is never updated nowYou removed CPU sampling but kept the gauge; it will stay at 0 unless
monitorCPUUsageis called elsewhere. Either wire minimal sampling or drop the gauge.Apply this diff to sample CPU when Electron’s
process.getCPUUsageexists:const memUsage = process.memoryUsage(); const memMB = Math.round(memUsage.rss / 1024 / 1024); this.monitorMemoryUsage(memMB); - // CPU usage monitoring (simplified) - // This is a simplified CPU calculation - in practice you'd want more sophisticated monitoring - + // CPU usage monitoring (Electron provides getCPUUsage) + try { + if (typeof process.getCPUUsage === 'function') { + const cpu = process.getCPUUsage(); + if (cpu && typeof cpu.percentCPUUsage === 'number') { + this.monitorCPUUsage(Math.round(cpu.percentCPUUsage)); + } + } + } catch (_) {}src/renderer/src/components/Chat/Poll.jsx (1)
193-216: Avoid shadowinghasVoted; clarify per-option stateShadowing the outer
hasVotedwith a localhasVotedis confusing. Use a distinct name.Apply this diff:
- const hasVoted = pollDetails?.voted_option_id === option.id; + const votedThisOption = pollDetails?.voted_option_id === option.id; @@ - {hasVoted && <span id="pollOptionVotedLabel">VOTED</span>} + {votedThisOption && <span id="pollOptionVotedLabel">VOTED</span>}src/renderer/src/components/Dialogs/User.jsx (1)
28-36: Remove write‑only state used only to force re‑renders
[, setSilencedUsers] = useState(...)is an anti‑pattern here; you already drive UI viaisUserSilenced. Remove this state and its setters.Apply this diff:
- const [, setSilencedUsers] = useState(() => { - try { - return JSON.parse(localStorage.getItem("silencedUsers")) || { data: [] }; - } catch (e) { - console.error("Error parsing silenced users:", e); - return { data: [] }; - } - });And remove these calls later:
- setSilencedUsers(silencedUsersData);- setSilencedUsers(currentSilencedUsers);src/renderer/src/components/Chat/Pin.jsx (1)
16-36: Good error handling improvements, but consider undefined access patterns.The new
handleUnpinClickfunction adds proper error handling and guards. However, the optional chaining patternwindow?.app?.kick?.getUnpinMessage?.()could mask issues where the API is unexpectedly unavailable.Consider adding a more explicit check with informative error logging:
const handleUnpinClick = async () => { if (!canModerate || !chatroomName) return; try { + if (!window.app?.kick?.getUnpinMessage) { + console.warn('[Chat][Pin] Unpin API not available'); + return; + } - const response = await window?.app?.kick?.getUnpinMessage?.(chatroomName); + const response = await window.app.kick.getUnpinMessage(chatroomName); if (response?.code === 201) {utils/services/seventv/sharedStvWebSocket.js (1)
78-80: Duplicate code detected between stvWebsocket.js and sharedStvWebSocket.js.The same backgroundImage conditional logic appears in both files. This duplication could lead to maintenance issues if the logic needs to change.
Consider extracting the gradient background generation logic into a shared utility function:
// In a new file: utils/services/seventv/gradientUtils.js export function generateGradientBackground(data, isDeg_or_Shape, gradient, randomColor) { if (data.function || data.style) { return `${data.function || "linear-gradient"}(${isDeg_or_Shape}, ${gradient})`; } return `${data.style || "linear-gradient"}(${data.shape || ""} 0deg, ${randomColor}, ${randomColor})`; }Then use it in both files to eliminate duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
donation-replay.js(0 hunks)eslint.config.js(1 hunks)scripts/event-collection/event-analyzer.js(1 hunks)scripts/event-collection/event-collector.js(2 hunks)scripts/test-telemetry-settings.js(3 hunks)src/main/index.js(2 hunks)src/preload/index.js(1 hunks)src/renderer/src/components/Chat/Input/EmoteDialogs.jsx(0 hunks)src/renderer/src/components/Chat/Input/index.jsx(1 hunks)src/renderer/src/components/Chat/Pin.jsx(2 hunks)src/renderer/src/components/Chat/Poll.jsx(2 hunks)src/renderer/src/components/Dialogs/Mentions.jsx(0 hunks)src/renderer/src/components/Dialogs/Settings.jsx(1 hunks)src/renderer/src/components/Dialogs/User.jsx(2 hunks)src/renderer/src/components/Messages/ModActions.jsx(1 hunks)src/renderer/src/components/Messages/ReplyMessage.jsx(0 hunks)src/renderer/src/providers/ChatProvider.jsx(6 hunks)src/renderer/src/telemetry/webTracing.js(8 hunks)src/renderer/src/utils/MessageParser.jsx(0 hunks)src/telemetry/error-monitoring.js(2 hunks)src/telemetry/performance-budget.js(1 hunks)src/telemetry/prometheus-server.js(1 hunks)src/telemetry/user-analytics.js(4 hunks)utils/fetchQueue.js(1 hunks)utils/services/connectionManager.js(1 hunks)utils/services/kick/kickAPI.js(1 hunks)utils/services/kick/sharedKickPusher.js(1 hunks)utils/services/seventv/sharedStvWebSocket.js(3 hunks)utils/services/seventv/stvAPI.js(0 hunks)utils/services/seventv/stvWebsocket.js(4 hunks)
💤 Files with no reviewable changes (6)
- src/renderer/src/components/Chat/Input/EmoteDialogs.jsx
- src/renderer/src/components/Messages/ReplyMessage.jsx
- src/renderer/src/utils/MessageParser.jsx
- donation-replay.js
- utils/services/seventv/stvAPI.js
- src/renderer/src/components/Dialogs/Mentions.jsx
🧰 Additional context used
📓 Path-based instructions (9)
**
📄 CodeRabbit inference engine (AGENTS.md)
**: Use electron-vite + React conventions instead of raw Electron patterns
When unsure, consult electron-vite/Electron/Vite docs (Context7) to confirm patterns
Omit empty sections in release notes
Files:
scripts/event-collection/event-analyzer.jssrc/renderer/src/components/Messages/ModActions.jsxsrc/renderer/src/components/Chat/Input/index.jsxutils/services/connectionManager.jsscripts/event-collection/event-collector.jssrc/telemetry/prometheus-server.jssrc/renderer/src/components/Chat/Poll.jsxsrc/renderer/src/components/Chat/Pin.jsxsrc/renderer/src/components/Dialogs/Settings.jsxsrc/telemetry/error-monitoring.jsutils/fetchQueue.jseslint.config.jssrc/main/index.jsutils/services/kick/sharedKickPusher.jsutils/services/kick/kickAPI.jssrc/telemetry/performance-budget.jsutils/services/seventv/stvWebsocket.jssrc/telemetry/user-analytics.jssrc/renderer/src/components/Dialogs/User.jsxsrc/renderer/src/telemetry/webTracing.jsutils/services/seventv/sharedStvWebSocket.jssrc/preload/index.jsscripts/test-telemetry-settings.jssrc/renderer/src/providers/ChatProvider.jsx
!dist/**
📄 CodeRabbit inference engine (AGENTS.md)
Do not commit built installers/artifacts in dist/
Files:
scripts/event-collection/event-analyzer.jssrc/renderer/src/components/Messages/ModActions.jsxsrc/renderer/src/components/Chat/Input/index.jsxutils/services/connectionManager.jsscripts/event-collection/event-collector.jssrc/telemetry/prometheus-server.jssrc/renderer/src/components/Chat/Poll.jsxsrc/renderer/src/components/Chat/Pin.jsxsrc/renderer/src/components/Dialogs/Settings.jsxsrc/telemetry/error-monitoring.jsutils/fetchQueue.jseslint.config.jssrc/main/index.jsutils/services/kick/sharedKickPusher.jsutils/services/kick/kickAPI.jssrc/telemetry/performance-budget.jsutils/services/seventv/stvWebsocket.jssrc/telemetry/user-analytics.jssrc/renderer/src/components/Dialogs/User.jsxsrc/renderer/src/telemetry/webTracing.jsutils/services/seventv/sharedStvWebSocket.jssrc/preload/index.jsscripts/test-telemetry-settings.jssrc/renderer/src/providers/ChatProvider.jsx
src/renderer/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/renderer/src/**/*.{ts,tsx,js,jsx}: Do not use direct Node APIs in the renderer
Use ipcRenderer.invoke('channel', payload) for request/response from renderer
Use import.meta.env.RENDERER_VITE_* for renderer-scoped config
Avoid bare KT_* env vars in the renderer; use RENDERER_VITE_KT_* instead
Files:
src/renderer/src/components/Messages/ModActions.jsxsrc/renderer/src/components/Chat/Input/index.jsxsrc/renderer/src/components/Chat/Poll.jsxsrc/renderer/src/components/Chat/Pin.jsxsrc/renderer/src/components/Dialogs/Settings.jsxsrc/renderer/src/components/Dialogs/User.jsxsrc/renderer/src/telemetry/webTracing.jssrc/renderer/src/providers/ChatProvider.jsx
src/{renderer/src,preload}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Resolve static assets via Vite URLs (new URL('./asset', import.meta.url))
Files:
src/renderer/src/components/Messages/ModActions.jsxsrc/renderer/src/components/Chat/Input/index.jsxsrc/renderer/src/components/Chat/Poll.jsxsrc/renderer/src/components/Chat/Pin.jsxsrc/renderer/src/components/Dialogs/Settings.jsxsrc/renderer/src/components/Dialogs/User.jsxsrc/renderer/src/telemetry/webTracing.jssrc/preload/index.jssrc/renderer/src/providers/ChatProvider.jsx
src/{main,renderer/src}/**/*.{ts,tsx,js,jsx,mts,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
For fire-and-forget events, use ipcRenderer.send and ipcMain.on with namespaced channels like 'app:settings:get'
Files:
src/renderer/src/components/Messages/ModActions.jsxsrc/renderer/src/components/Chat/Input/index.jsxsrc/renderer/src/components/Chat/Poll.jsxsrc/renderer/src/components/Chat/Pin.jsxsrc/renderer/src/components/Dialogs/Settings.jsxsrc/main/index.jssrc/renderer/src/components/Dialogs/User.jsxsrc/renderer/src/telemetry/webTracing.jssrc/renderer/src/providers/ChatProvider.jsx
src/{main,preload,renderer/src}/**/*.{ts,tsx,js,jsx,mts,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use shared non-secret variables via import.meta.env.VITE_*
Files:
src/renderer/src/components/Messages/ModActions.jsxsrc/renderer/src/components/Chat/Input/index.jsxsrc/renderer/src/components/Chat/Poll.jsxsrc/renderer/src/components/Chat/Pin.jsxsrc/renderer/src/components/Dialogs/Settings.jsxsrc/main/index.jssrc/renderer/src/components/Dialogs/User.jsxsrc/renderer/src/telemetry/webTracing.jssrc/preload/index.jssrc/renderer/src/providers/ChatProvider.jsx
src/{telemetry,renderer/src/telemetry}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/{telemetry,renderer/src/telemetry}/**/*.{ts,tsx,js,jsx}: Place OpenTelemetry instrumentation under src/telemetry/** and src/renderer/src/telemetry/**
Configure OTLP HTTP export to Grafana Cloud; read endpoints/keys from env with main-safe prefixes; do not expose secrets to renderer
Files:
src/telemetry/prometheus-server.jssrc/telemetry/error-monitoring.jssrc/telemetry/performance-budget.jssrc/telemetry/user-analytics.jssrc/renderer/src/telemetry/webTracing.js
src/main/**/*.{ts,js,mts,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
src/main/**/*.{ts,js,mts,mjs,cjs}: From the main process, load the built preload bundle (e.g., join(dirname, '../preload/index.js') or fileURLToPath(new URL('../preload/index.js', import.meta.url)) in ESM)
Keep BrowserWindow webPreferences.contextIsolation: true
Validate and sanitize all inputs received from renderer in main process handlers
Use ipcMain.handle('channel', fn) for request/response handlers in main
Access secrets in main via process.env or MAIN_VITE*; never expose them to renderer
Use import.meta.env.MAIN_VITE* for main-scoped non-secret config
Files:
src/main/index.js
src/preload/**/*.{ts,js,mts,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
src/preload/**/*.{ts,js,mts,mjs,cjs}: Expose a minimal, purpose-built API from preload via contextBridge.exposeInMainWorld
Do not expose raw Node objects/APIs from preload
Use import.meta.env.PRELOAD_VITE_* for preload-scoped config
Files:
src/preload/index.js
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/{telemetry,renderer/src/telemetry}/**/*.{ts,tsx,js,jsx} : Configure OTLP HTTP export to Grafana Cloud; read endpoints/keys from env with main-safe prefixes; do not expose secrets to renderer
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/{telemetry,renderer/src/telemetry}/**/*.{ts,tsx,js,jsx} : Place OpenTelemetry instrumentation under src/telemetry/** and src/renderer/src/telemetry/**
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/{telemetry,renderer/src/telemetry}/**/*.{ts,tsx,js,jsx} : Configure OTLP HTTP export to Grafana Cloud; read endpoints/keys from env with main-safe prefixes; do not expose secrets to renderer
Applied to files:
src/renderer/src/telemetry/webTracing.js
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/{telemetry,renderer/src/telemetry}/**/*.{ts,tsx,js,jsx} : Place OpenTelemetry instrumentation under src/telemetry/** and src/renderer/src/telemetry/**
Applied to files:
src/renderer/src/telemetry/webTracing.js
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/main/**/*.{ts,js,mts,mjs,cjs} : Validate and sanitize all inputs received from renderer in main process handlers
Applied to files:
src/renderer/src/telemetry/webTracing.js
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/preload/**/*.{ts,js,mts,mjs,cjs} : Expose a minimal, purpose-built API from preload via contextBridge.exposeInMainWorld
Applied to files:
src/preload/index.js
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,js,jsx} : Use ipcRenderer.invoke('channel', payload) for request/response from renderer
Applied to files:
src/preload/index.js
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to {**/__tests__/**/*.{ts,tsx,js,jsx},**/*.test.@(ts|tsx|js|jsx)} : Mock Electron bridges in tests to keep them fast and deterministic
Applied to files:
src/preload/index.jsscripts/test-telemetry-settings.js
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/{main,renderer/src}/**/*.{ts,tsx,js,jsx,mts,mjs,cjs} : For fire-and-forget events, use ipcRenderer.send and ipcMain.on with namespaced channels like 'app:settings:get'
Applied to files:
src/preload/index.js
📚 Learning: 2025-09-18T22:47:22.731Z
Learnt from: CR
PR: BP602/KickTalk#0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T22:47:22.731Z
Learning: Applies to src/main/**/*.{ts,js,mts,mjs,cjs} : Use ipcMain.handle('channel', fn) for request/response handlers in main
Applied to files:
src/preload/index.js
🧬 Code graph analysis (6)
src/telemetry/performance-budget.js (3)
src/telemetry/error-monitoring.js (2)
require(2-2)require(3-3)src/telemetry/user-analytics.js (1)
require(2-2)src/telemetry/metrics.js (6)
require(2-2)require(3-3)require(4-4)require(5-5)require(6-6)require(7-7)
utils/services/seventv/stvWebsocket.js (1)
utils/services/seventv/sharedStvWebSocket.js (5)
data(21-21)data(39-39)isDeg_or_Shape(67-67)gradient(59-59)randomColor(49-49)
src/telemetry/user-analytics.js (3)
src/telemetry/error-monitoring.js (2)
require(2-2)require(3-3)src/telemetry/metrics.js (6)
require(2-2)require(3-3)require(4-4)require(5-5)require(6-6)require(7-7)src/renderer/src/telemetry/userAnalyticsHelper.js (2)
FEATURE_CATEGORIES(35-41)FEATURE_CATEGORIES(35-41)
utils/services/seventv/sharedStvWebSocket.js (1)
utils/services/seventv/stvWebsocket.js (6)
data(17-17)data(37-37)data(130-130)isDeg_or_Shape(65-65)gradient(57-57)randomColor(47-47)
scripts/test-telemetry-settings.js (2)
src/telemetry/tracing.js (2)
store(16-16)path(38-38)scripts/verify-telemetry-checks.js (2)
path(9-9)results(42-42)
src/renderer/src/providers/ChatProvider.jsx (2)
src/renderer/src/utils/chatErrors.js (2)
chatroomErrorHandler(3-6)chatroomErrorHandler(3-6)src/renderer/src/providers/CosmeticsProvider.jsx (1)
useCosmeticsStore(3-85)
🪛 Biome (2.1.2)
src/renderer/src/components/Chat/Poll.jsx
[error] 38-38: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-builds (windows-latest)
🔇 Additional comments (37)
src/renderer/src/components/Messages/ModActions.jsx (1)
1-1: LGTM! Clean unused import removal.The removal of the unused
useEffectimport is appropriate and aligns with the ESLint rule enhancements in this PR that enforce proper import usage.src/renderer/src/components/Chat/Input/index.jsx (1)
1040-1040: LGTM! Consistent naming convention for unused catch parameters.The renaming from
eto_efollows the codebase convention for unused catch parameters and aligns with similar changes throughout the PR.scripts/event-collection/event-analyzer.js (1)
70-70: LGTM! Consistent unused catch parameter naming.The change from
errorto_errorfollows the established pattern across the PR for indicating unused catch parameters, while maintaining the same error handling behavior.src/telemetry/error-monitoring.js (2)
149-149: LGTM! Consistent unused parameter naming.The change from
fallbackErrorto_fallbackErrorappropriately indicates that this catch parameter is not used in the error handling logic.
432-432: LGTM! Consistent unused parameter naming.The change from
fallbackErrorto_fallbackErrormaintains consistency with the pattern established elsewhere in this file and across the codebase.utils/services/connectionManager.js (1)
9-9: LGTM! Consistent unused parameter naming.The change from
eto_efollows the established convention for unused catch parameters in OpenTelemetry initialization blocks throughout the codebase.src/telemetry/prometheus-server.js (1)
20-20: LGTM! Consistent unused parameter naming.The change from
errorto_errorfollows the established pattern for unused catch parameters while maintaining the same fallback behavior.utils/services/kick/sharedKickPusher.js (1)
6-6: LGTM! Consistent unused parameter naming.The change from
eto_ein the OpenTelemetry initialization follows the established pattern across the codebase for indicating unused catch parameters.scripts/event-collection/event-collector.js (2)
131-134: Destructuring cleanup is correctIgnoring the unused Map key is idiomatic and removes lint noise. No functional change.
216-235: Destructuring cleanup is correctSame here; subscription behavior unchanged.
src/renderer/src/components/Dialogs/User.jsx (1)
7-7: Import cleanup is fineDropping
KickTalkBadgesimport is consistent with usage.src/preload/index.js (1)
1-1: Import cleanup approved — removing electron 'session' from preload is safesrc/preload/index.js no longer imports session and only uses local "session" identifiers (auth tokens/properties); no electron.session references remain in preload. electron.session is still used in src/main/index.js (expected).
eslint.config.js (1)
92-95: Enable React Hooks ESLint rulesAdd the react-hooks rules to the renderer rules to catch conditional-hook and deps issues.
Location: eslint.config.js — inside the renderer config (files: ["src/renderer/**"]).
Apply this diff:
// Essential React rules to properly detect JSX usage "react/jsx-uses-react": "error", "react/jsx-uses-vars": "error", + // Hooks correctness + "react-hooks/rules-of-hooks": "error", + "react-hooks/exhaustive-deps": "warn",After applying, run: npm run lint --silent and fix any reported issues (I couldn't run lint in this environment — eslint not found).
src/renderer/src/components/Chat/Pin.jsx (1)
60-64: LGTM! Clean async wrapper pattern.The use of
void handleUnpinClick()properly handles the async function without creating floating promises, which is a good ESLint compliance fix.utils/services/seventv/stvWebsocket.js (2)
89-91: Consistent conditional pattern applied to animated backgrounds.The animated background logic mirrors the non-animated changes, properly using
data.image_urlwhen available.
197-211: Comments improve code clarity.The addition of contextual comments (
// No STV ID,// No emote set ID,// No channel kick ID) helps clarify the empty blocks' purpose, which is good for maintainability.utils/services/seventv/sharedStvWebSocket.js (1)
141-141: Unused variable naming follows best practices.Renaming the catch parameter to
_eproperly indicates it's unused, which is a good ESLint compliance fix.src/renderer/src/providers/ChatProvider.jsx (4)
695-695: Unused variable naming convention applied.The error message variable is renamed to
___to indicate it's unused, properly addressing the ESLint warning.Also applies to: 827-827
910-916: Block scoping for case statements improves clarity.Adding braces to create scoped blocks for the entitlement.create cases is good practice and prevents potential variable hoisting issues.
Also applies to: 1906-1911
1100-1189: Case block scoping consistently applied.The addition of braces around the ChatMessageEvent case creates a proper scope, which is consistent with the other changes and good for code organization.
380-381: Approve — removing default destructured fields is safeVerified: pinDetails/pollDetails are always guarded before use (Pin.jsx, Poll.jsx, StreamerInfo.jsx); chatters are tracked separately in state.chatters and accessed with fallbacks (ChatProvider.addChatter, Input/index.jsx, Dialogs/Chatters.jsx). The destructuring intentionally strips transient fields from persisted chatrooms; no code depends on the removed defaults.
scripts/test-telemetry-settings.js (6)
16-19: Cross-platform compatibility and configurable timeouts.Good improvements for cross-platform support with
npm.cmdon Windows and configurable timeouts via environment variables. The default values are reasonable.
22-30: Excellent telemetry restoration logic.The
restoreTelemetrySettingfunction properly handles both defined and undefined original values, ensuring the telemetry state is correctly restored after tests. This prevents test side effects.
32-76: Robust process management with proper cleanup.The
runAppAndCollectLogsfunction implements excellent process lifecycle management:
- Graceful shutdown with SIGTERM
- Fallback to SIGKILL after timeout
- Proper error handling and log collection
78-121: Test functions properly refactored to use the new helper.Both test functions now use the centralized
runAppAndCollectLogshelper, and the assertion logic usingObject.values(checks).every(Boolean)is clean and consistent.
159-164: Conditional test execution with clear messaging.The SKIP_RUNTIME_TESTS flag allows bypassing potentially slow runtime tests, with a clear warning message when skipped.
184-186: Proper cleanup in finally block.Using
process.exitCodeinstead ofprocess.exit(1)and ensuring telemetry restoration in the finally block are both best practices.src/main/index.js (1)
2198-2208: Confirm the consistency of underscore prefixes for unused catch parameters.The catch block parameters were renamed from
pathError/errorto_pathError/_errorto indicate they're unused, which aligns with the repository-wide convention. This is consistent with similar changes in other files.src/telemetry/user-analytics.js (3)
2-2: LGTM! Import cleanup aligns with telemetry architecture.Removing the unused
ErrorMonitorimport and context binding helps reduce the module's surface area without affecting functionality.
522-523: LGTM! Consistent error parameter naming convention.The catch block parameters renamed to
_errorand the loop destructuring change from[userId, features]to[userId](dropping unusedfeatures) both follow the established pattern of prefixing unused variables with underscores.Also applies to: 535-535
155-155: Approve — destructuring intentionally ignores the key; logic preserved.
getFeatureFromAction only reads config.actions and config.name; there are no other references to acategoryidentifier in this file and this is the only occurrence of that ignored-key destructuring. Location: src/telemetry/user-analytics.js:155.src/renderer/src/telemetry/webTracing.js (5)
8-8: IPC-based telemetry export properly implements security requirements.The migration from SimpleSpanProcessor to the custom IPCSpanExporter/IPCSimpleSpanProcessor correctly implements the IPC-based export path. This aligns with the learned requirement that renderer should not have direct access to OTLP endpoints/secrets, instead relaying through the main process via IPC.
Also applies to: 1030-1102
461-461: Sanitization function naming avoids scope collision.Renaming the sanitization helpers to
___sanitizeAnyValueand___sanitizeAttributes(triple underscores) effectively avoids potential naming conflicts across different scopes while maintaining consistency.Also applies to: 773-773, 808-820
704-726: Robust time handling with BigInt for nanosecond precision.The implementation correctly handles HRTime to epoch nanosecond conversion using BigInt for precision, with proper fallbacks and origin-based calculations. The conditional logic to distinguish between epoch-based and relative timestamps is well-reasoned.
379-606: Comprehensive transport interceptors for OTLP relay.The implementation provides thorough coverage of all browser transport methods (fetch, XHR, sendBeacon) to intercept calls to the sentinel OTLP URL and relay them via IPC. The interceptors properly handle various body formats (ArrayBuffer, Blob, string/JSON) with appropriate error handling.
1948-2024: Approve — verify renderer contains no direct OTLP env usageTelemetry test scaffolding and IPC export look good. The ripgrep returned no matches for OTEL_EXPORTER_OTLP_ENDPOINT/HEADERS — absence of matches is not proof; confirm renderer does not reference OTLP endpoint/headers and that OTLP endpoints/keys are loaded only in main using main-safe env prefixes (do not expose secrets to renderer). Ref: src/renderer/src/telemetry/webTracing.js (lines ~1948–2024).
src/renderer/src/components/Dialogs/Settings.jsx (1)
19-19: Correctly removing unused state variable.The change to ignore the first element of the
useStatedestructuring is appropriate sinceuserDatawas never read after being set. The settersetUserDatais still called on line 26, maintaining the expected behavior while eliminating the ESLint warning for unused variables.
| if (!pollDetails?.title) return null; | ||
|
|
||
| const [poll, setPoll] = useState(pollDetails); | ||
| const [percentRemaining, setPercentRemaining] = useState(0); | ||
|
|
||
| const [selectedOption, setSelectedOption] = useState(pollDetails?.voted_option_id || null); | ||
| const [hasVoted, setHasVoted] = useState(pollDetails?.has_voted || false); | ||
| const [hasVoted] = useState(pollDetails?.has_voted || false); | ||
| const [pollWinner, setPollWinner] = useState(null); | ||
| const [isPollExpanded, setIsPollExpanded] = useState(false); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix rules-of-hooks violation (early return before hooks)
if (!pollDetails?.title) return null; precedes hooks. Hooks must run unconditionally across renders.
Apply this diff to move the guard after hooks:
- if (!pollDetails?.title) return null;
-
const [percentRemaining, setPercentRemaining] = useState(0);
@@
const [isPollExpanded, setIsPollExpanded] = useState(false);
+ // Guard after hooks
+ if (!pollDetails?.title) return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!pollDetails?.title) return null; | |
| const [poll, setPoll] = useState(pollDetails); | |
| const [percentRemaining, setPercentRemaining] = useState(0); | |
| const [selectedOption, setSelectedOption] = useState(pollDetails?.voted_option_id || null); | |
| const [hasVoted, setHasVoted] = useState(pollDetails?.has_voted || false); | |
| const [hasVoted] = useState(pollDetails?.has_voted || false); | |
| const [pollWinner, setPollWinner] = useState(null); | |
| const [isPollExpanded, setIsPollExpanded] = useState(false); | |
| const [percentRemaining, setPercentRemaining] = useState(0); | |
| const [selectedOption, setSelectedOption] = useState(pollDetails?.voted_option_id || null); | |
| const [hasVoted] = useState(pollDetails?.has_voted || false); | |
| const [pollWinner, setPollWinner] = useState(null); | |
| const [isPollExpanded, setIsPollExpanded] = useState(false); | |
| // Guard after hooks | |
| if (!pollDetails?.title) return null; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 35-35: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 37-37: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 38-38: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 39-39: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 40-40: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In src/renderer/src/components/Chat/Poll.jsx around lines 33 to 41, the early
return guard (`if (!pollDetails?.title) return null;`) appears before hook calls
causing a rules-of-hooks violation; move that guard to after all useState (and
any other hook) calls so hooks always run in the same order, and ensure any
state initializers that use pollDetails use safe defaults (e.g., null/false) so
they don't depend on pollDetails being present during initial render; keep the
component behavior unchanged otherwise.
| const { input, resolve } = fetchQueue.shift(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Do not swallow errors; preserve reject path
Changing the contract to always resolve (with the string "error") will break callers expecting rejection and forces brittle string checks.
Apply this diff to restore the promise rejection path and improve telemetry status reporting:
- const { input, resolve } = fetchQueue.shift();
+ const { input, resolve, reject } = fetchQueue.shift();- } catch (error) {
+ } catch (error) {
console.error("Error fetching data:", error);
@@
- await window.app?.telemetry?.recordAPIRequest?.('kick_get_channel_info', 'GET', 500, duration);
+ const status = error?.response?.status ?? 500;
+ await window.app?.telemetry?.recordAPIRequest?.('kick_get_channel_info', 'GET', status, duration);
} catch (_) {}
-
- resolve("error");
+ reject(error);Run to find usages that assume resolve-on-error behavior or rely on rejection:
🏁 Script executed:
#!/bin/bash
rg -nC2 '\bqueueChannelFetch\(' --type=js --type=jsxLength of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for queueChannelFetch usages..."
rg -nC2 -S '\bqueueChannelFetch\s*\(' || true
echo "Searching for fetchQueue symbol occurrences..."
rg -nC2 -S '\bfetchQueue\b' || true
echo "Searching for resolve(\"error\") occurrences..."
rg -nC2 -S 'resolve\(\s*"?error"?\s*\)' || true
echo "Attempting to show utils/fetchQueue.js if present..."
if [ -f utils/fetchQueue.js ]; then
echo "=== utils/fetchQueue.js ==="
sed -n '1,240p' utils/fetchQueue.js
else
echo "utils/fetchQueue.js not found at utils/. Listing files matching 'fetchQueue'..."
rg -n --hidden -S 'fetchQueue' || true
fiLength of output: 3194
Do not swallow errors; preserve reject path
File: utils/fetchQueue.js (processQueue, lines ~8–9). Callers (e.g., src/renderer/src/providers/ChatProvider.jsx:2128) await queueChannelFetch and expect rejections — resolving "error" hides failures and forces brittle string checks. Restore the reject path and surface the real HTTP status in telemetry.
- const { input, resolve } = fetchQueue.shift();
+ const { input, resolve, reject } = fetchQueue.shift();- } catch (error) {
+ } catch (error) {
console.error("Error fetching data:", error);
@@
- await window.app?.telemetry?.recordAPIRequest?.('kick_get_channel_info', 'GET', 500, duration);
+ const status = error?.response?.status ?? 500;
+ await window.app?.telemetry?.recordAPIRequest?.('kick_get_channel_info', 'GET', status, duration);
} catch (_) {}
-
- resolve("error");
+ reject(error);📝 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 { input, resolve } = fetchQueue.shift(); | |
| const { input, resolve, reject } = fetchQueue.shift(); | |
| try { | |
| // ... existing fetch logic ... | |
| } catch (error) { | |
| console.error("Error fetching data:", error); | |
| try { | |
| const duration = Date.now() - start; | |
| const status = error?.response?.status ?? 500; | |
| await window.app?.telemetry?.recordAPIRequest?.('kick_get_channel_info', 'GET', status, duration); | |
| } catch (_) {} | |
| reject(error); | |
| } |
🤖 Prompt for AI Agents
In utils/fetchQueue.js around lines 8–9, the code currently destructures only
input and resolve from fetchQueue.shift() and resolves error results as values;
instead destructure the reject function as well (e.g., const { input, resolve,
reject } = fetchQueue.shift()), and when the fetch fails call reject(error) so
the original promise is rejected (do not resolve with an "error" string). Also
propagate HTTP status information from the response/error (e.g., include
response.status or error.status) into telemetry before calling reject so callers
receive the real failure and telemetry contains the status.
| console.error('Poll vote error:', error); | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid logging tokens/cookies in axios error objects
console.error('Poll vote error:', error) can print error.config.headers and error.request, which include Authorization and Cookie (session secrets). Log only safe fields.
Apply this diff:
- console.error('Poll vote error:', error);
+ console.error('[KickAPI] Poll vote error:', {
+ message: error?.message,
+ status: error?.response?.status,
+ code: error?.code
+ });📝 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.
| console.error('Poll vote error:', error); | |
| throw error; | |
| } | |
| console.error('[KickAPI] Poll vote error:', { | |
| message: error?.message, | |
| status: error?.response?.status, | |
| code: error?.code | |
| }); | |
| throw error; | |
| } |
🤖 Prompt for AI Agents
In utils/services/kick/kickAPI.js around lines 540 to 542, the current
console.error('Poll vote error:', error) can leak sensitive headers/cookies from
error.config.headers and error.request; replace this direct error dump with a
sanitized log that only includes safe fields (e.g., error.message, error.stack,
error.response?.status, and error.response?.data) or construct a shallow object
omitting config and request (or explicitly deleting headers/cookies) before
logging; ensure you do not log error.config, error.config.headers, or
error.request, and prefer a structured logger call with the sanitized object.
| backgroundImage: data.function || data.style | ||
| ? `${data.function || "linear-gradient"}(${isDeg_or_Shape}, ${gradient})` | ||
| : `${data.style || "linear-gradient"}(${data.shape || ""} 0deg, ${randomColor}, ${randomColor})`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Conditional logic change may alter gradient behavior.
The backgroundImage logic has been changed from an OR-based construction to a conditional. The new pattern prioritizes data.function || data.style over the fallback, which could change the gradient rendering when both conditions evaluate differently than before.
Please verify that this change maintains the intended gradient rendering behavior:
🏁 Script executed:
#!/bin/bash
# Search for other uses of similar gradient patterns to ensure consistency
rg -n "backgroundImage.*gradient" --type jsLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking for target file..."
if [ -f utils/services/seventv/stvWebsocket.js ]; then
echo "FOUND: utils/services/seventv/stvWebsocket.js"
echo "----- file head (first 240 lines) -----"
sed -n '1,240p' utils/services/seventv/stvWebsocket.js || true
else
echo "File not found at utils/services/seventv/stvWebsocket.js — searching repository for 'stvWebsocket'..."
rg -n --hidden --glob '!node_modules' "stvWebsocket" || true
fi
echo "----- searching for backgroundImage occurrences -----"
rg -n --hidden --glob '!node_modules' "backgroundImage" --type js || true
echo "----- searching for 'data.function' or 'data.style' usages -----"
rg -n --hidden --glob '!node_modules' "data\.function|data\.style" --type js || true
echo "----- searching for other gradient constructions -----"
rg -n --hidden --glob '!node_modules' "linear-gradient|gradient\\(" --type js || trueLength of output: 7708
Incorrect fallback: data.style is ignored when data.function is falsy
The ternary checks (data.function || data.style) but the constructed string uses (data.function || "linear-gradient"), so when data.style exists and data.function is falsy you end up using "linear-gradient" instead of data.style (and isDeg_or_Shape is computed off data.function). Fix by selecting a single fn = data.function || data.style || "linear-gradient", derive isDeg_or_Shape from that fn, and use ${fn}(${isDeg_or_Shape}, ${gradient}).
Location: utils/services/seventv/stvWebsocket.js — backgroundImage assignment in the PAINT handling (push object).
🤖 Prompt for AI Agents
In utils/services/seventv/stvWebsocket.js around lines 76 to 78, the ternary
chooses based on (data.function || data.style) but then uses (data.function ||
"linear-gradient") and computes isDeg_or_Shape from data.function, causing
data.style to be ignored when data.function is falsy; fix by computing one fn =
data.function || data.style || "linear-gradient", compute isDeg_or_Shape based
on that fn (instead of data.function), and use `${fn}(${isDeg_or_Shape},
${gradient})` in the backgroundImage branch so the correct fallback is used.
* Fix cross-platform icon compatibility - Add cross-platform icon handling for Linux/macOS support - Convert Windows .ico to .png for non-Windows platforms - Update tray and thumbar icons to use platform-appropriate format Fixes application crashes on Linux due to unsupported .ico format 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix(chat): correct chatters selector for @ mention autocomplete Fixes the data selector for chatters in the chat input component. The selector was looking for chatters in state.chatrooms[].chatters but the data is actually stored in state.chatters[chatroomId]. Resolves #40 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
Key Changes
ESLint Configuration Fix
react/jsx-uses-reactreact/jsx-uses-varsSecurity Updates
Code Cleanup
Test Plan
npm run dev)npm auditclean)Files Modified
31 files across components, telemetry, utilities, and configuration:
Risk Assessment
Low risk - Changes are primarily:
Summary by CodeRabbit