Feature/rpc monitoring and UI optimization#29
Conversation
- Register @fastify/static to serve public/ at /ui/ endpoint - Update Content-Security-Policy to allow unpkg CDN, Google Fonts, and GitHub raw - Extract countChainsByTag helper to dataService.js for reuse - Consolidate fileURLToPath imports Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevents TypeError when detail panel elements are missing from the DOM, which occurs when HTML and JS versions are mismatched during deployment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds support for serving a bundled 3D visualization UI via Fastify static hosting, refactors stats counting into a shared helper, and hardens the UI client code against missing DOM elements.
Changes:
- Serve
public/assets under/ui/via@fastify/static, and expand CSP directives to support external UI resources. - Refactor chain stats counting into a reusable
countChainsByTag()helper used by/stats. - Add null-guards in the UI JS to avoid runtime errors when expected DOM nodes are missing.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| public/app.js | Adds defensive DOM lookups/guards to prevent UI TypeErrors. |
| index.js | Registers @fastify/static for /ui/, adjusts CSP directives, and uses shared chain-tag counting for /stats. |
| dataService.js | Introduces countChainsByTag() helper for consistent chain category counts. |
| package.json | Adds @fastify/static dependency for static file serving. |
| package-lock.json | Locks new transitive dependencies pulled in by @fastify/static. |
You can also share your feedback on Copilot code review. Take the survey.
index.js
Outdated
| scriptSrc: ["'self'", "https://unpkg.com"], | ||
| styleSrc: ["'self'", "'unsafe-inline'", "https://fonts.googleapis.com"], | ||
| fontSrc: ["'self'", "https://fonts.gstatic.com"], |
There was a problem hiding this comment.
CSP is being relaxed to allow third-party scripts (unpkg.com) and style-src 'unsafe-inline'. This meaningfully weakens XSS protections for the entire app (including API responses) since Helmet applies globally. Consider serving these JS/CSS assets locally (via /ui/) and removing third-party origins/unsafe-inline, or at least narrowing script-src to specific URLs with SRI/nonces/hashes.
| scriptSrc: ["'self'", "https://unpkg.com"], | |
| styleSrc: ["'self'", "'unsafe-inline'", "https://fonts.googleapis.com"], | |
| fontSrc: ["'self'", "https://fonts.gstatic.com"], | |
| scriptSrc: ["'self'"], | |
| styleSrc: ["'self'"], | |
| fontSrc: ["'self'"], |
| "@fastify/cors": "^11.2.0", | ||
| "@fastify/helmet": "^13.0.2", | ||
| "@fastify/rate-limit": "^10.3.0", | ||
| "@fastify/static": "^9.0.0", | ||
| "@modelcontextprotocol/sdk": "^1.26.0", |
There was a problem hiding this comment.
Adding @fastify/static pulls in dependencies that require Node 20+ (e.g., lru-cache@11 via glob@13). Since package.json does not declare an engines.node constraint, installs on Node 18 may fail unexpectedly. Consider adding an explicit "engines": { "node": ">=20" } (or whatever minimum you support) to match the lockfile/Docker image.
dataService.js
Outdated
| const totalTestnets = chains.filter(c => c.tags?.includes('Testnet')).length; | ||
| const totalL2s = chains.filter(c => c.tags?.includes('L2')).length; | ||
| const totalBeacons = chains.filter(c => c.tags?.includes('Beacon')).length; | ||
| const totalMainnets = chains.filter(c => !c.tags?.includes('Testnet') && !c.tags?.includes('L2') && !c.tags?.includes('Beacon')).length; | ||
|
|
There was a problem hiding this comment.
countChainsByTag() runs 4-5 full Array.filter() passes over the same chains array. If /stats is called frequently, this adds avoidable overhead. Consider computing all counters in a single loop/reduce to keep the operation O(n) with one pass.
| const totalTestnets = chains.filter(c => c.tags?.includes('Testnet')).length; | |
| const totalL2s = chains.filter(c => c.tags?.includes('L2')).length; | |
| const totalBeacons = chains.filter(c => c.tags?.includes('Beacon')).length; | |
| const totalMainnets = chains.filter(c => !c.tags?.includes('Testnet') && !c.tags?.includes('L2') && !c.tags?.includes('Beacon')).length; | |
| let totalTestnets = 0; | |
| let totalL2s = 0; | |
| let totalBeacons = 0; | |
| let totalMainnets = 0; | |
| for (const chain of chains) { | |
| const tags = chain.tags || []; | |
| const isTestnet = tags.includes('Testnet'); | |
| const isL2 = tags.includes('L2'); | |
| const isBeacon = tags.includes('Beacon'); | |
| if (isTestnet) { | |
| totalTestnets += 1; | |
| } | |
| if (isL2) { | |
| totalL2s += 1; | |
| } | |
| if (isBeacon) { | |
| totalBeacons += 1; | |
| } | |
| if (!isTestnet && !isL2 && !isBeacon) { | |
| totalMainnets += 1; | |
| } | |
| } |
| export function countChainsByTag(chains) { | ||
| const totalChains = chains.length; | ||
| const totalTestnets = chains.filter(c => c.tags?.includes('Testnet')).length; | ||
| const totalL2s = chains.filter(c => c.tags?.includes('L2')).length; | ||
| const totalBeacons = chains.filter(c => c.tags?.includes('Beacon')).length; | ||
| const totalMainnets = chains.filter(c => !c.tags?.includes('Testnet') && !c.tags?.includes('L2') && !c.tags?.includes('Beacon')).length; | ||
|
|
||
| return { totalChains, totalMainnets, totalTestnets, totalL2s, totalBeacons }; | ||
| } |
There was a problem hiding this comment.
countChainsByTag() is new logic used by /stats, but there’s no targeted unit test coverage for it. Adding a small unit test suite (e.g., mixed tags, missing tags, and the “mainnet excludes Testnet/L2/Beacon” case) would help prevent regressions in stats reporting.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@Johnaverse I've opened a new pull request, #30, to work on those changes. Once the pull request is ready, I'll request review from you. |
…untChainsByTag, and unit tests Co-authored-by: Johnaverse <110527930+Johnaverse@users.noreply.github.com>
Harden CSP, bundle UI assets locally, add Node engines constraint, optimize countChainsByTag, add unit tests
Here's the PR description:
Summary
/statsendpoint@fastify/staticto serve the 3D visualization UI at/ui/, with updated CSP headers for Three.js, Google Fonts, and GitHub raw contentcountChainsByTag()helper, and fixed mainnet counting logic to correctly exclude L2 and Beacon chainsshowNodeDetailsto prevent TypeErrors during HTML/JS version mismatches'Inter'monospace fallback with proper system monospace font stackTest plan
/statsendpoint returns correct mainnet counts (excludes Testnet, L2, and Beacon chains)/ui/serves the 3D visualization page/rpc-monitorreturns resultsshowNodeDetailsdoesn't throw when detail panel elements are missing