feat(web): add HTTP MCP server for dashboard icon search - #2885
feat(web): add HTTP MCP server for dashboard icon search#2885gfrancodev wants to merge 11 commits into
Conversation
Expose /api/mcp with search_icons, get_icon, get_icon_url, and suggest_icon tools. Extract shared icon service with layered metadata cache, input validation, per-IP rate limiting, and unit tests.
Expand Vitest coverage for MCP handlers, icon services, and API helpers with 100% thresholds on scoped modules and shared test setup for server-only code.
Rename the edge request handler to the new proxy convention while preserving canonical host redirects, HTTPS enforcement, and tracking param stripping.
Mark icon services as server-only, cache PocketBase and GitHub fetches during static generation, fix OpenTelemetry instrumentation, and resolve lint/a11y issues across dashboard and icon UI components.
Update Next.js, Vitest, OpenTelemetry, and security overrides, and document the MCP endpoint in the web README.
ajnart
left a comment
ajnart
left a comment
There was a problem hiding this comment.
I haven't tested the changes yet but thank you for this PR !
Should / could we have some kind of advertisement for this feature inside of the web app itself ? Like a "new - add MCP" button with instructions for all compatible clients ?
I haven't checked out the whole PR yet so apologies if it's already there
|
I think afterwards I'll add https://posthog.com/docs/mcp-analytics so that we can have some stats of how people use the MCP |
|
Thanks @ajnart! No in-app entry point yet, only docs. I am adding an "Add to MCP client" button today (same pattern as "Search from browser") on the home, /icons, and icon detail pages, with copy/paste setup for Cursor, Claude Desktop, Windsurf, and mcp-remote. Icon pages will include a contextual example for the current icon. Re PostHog MCP Analytics: sounds great for tracking real tool usage on /api/mcp. I can add simple setup funnel events on the website if helpful. Happy to help wire up the server instrumentation whenever you are ready. Pushing to this PR shortly. |
Expose in-app MCP onboarding with client-specific config snippets, site favicon in serverInfo, image rendering fixes, and Playwright tests.
|
@ajnart as promised, the in-app MCP entry points are in. "Add to MCP client" on home, /icons, and icon detail, with setup instructions for the clients above + contextual examples on icon pages. Screenshots attached. |
|
Thanks, I will check it out later today. For now i'll let @coderabbitai review |
|
✅ Action performedReview finished.
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe change adds an MCP server for icon search, metadata lookup, URL generation, and suggestions. It adds MCP setup UI, icon-service caching and validation, request protection, image rendering updates, runtime configuration, documentation, and tests. ChangesMCP icon service
Rendering and platform updates
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPRoute
participant RateLimiter
participant MCPHandler
participant IconService
MCPClient->>MCPRoute: Send MCP request
MCPRoute->>RateLimiter: Check IP and tool-call quotas
RateLimiter->>MCPRoute: Allow or reject request
MCPRoute->>MCPHandler: Forward accepted request
MCPHandler->>IconService: Execute icon tool
IconService->>MCPHandler: Return icon data
MCPHandler->>MCPClient: Return MCP response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/submissions-data-table.tsx (1)
633-650: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the checkbox outside the row button.
The mobile row button contains an interactive
Checkbox, creating nested interactive controls. This can break keyboard focus and screen-reader semantics even withstopPropagation(). Render the checkbox as a sibling of a separate detail button instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/submissions-data-table.tsx` around lines 633 - 650, Restructure the mobile submission row around the button with onClick={() => setMobileDetailSubmission(submission)} so the selectable Checkbox is rendered outside that button as a sibling control. Preserve the existing selection state, toggleMobileSelection behavior, styling, and detail-button behavior while ensuring the checkbox and detail action are separate interactive elements.
🧹 Nitpick comments (11)
web/README.md (1)
53-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMCP documentation is duplicated across two files.
This section repeats the endpoint URL, tools table, Cursor/
mcp-remoteconfiguration, environment variables, and rate limits already documented inweb/docs/MCP.md. Keeping both in sync manually risks drift as the MCP server evolves. Consider trimming this section to a short summary that links toweb/docs/MCP.mdas the single source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/README.md` around lines 53 - 110, The MCP section in web/README.md duplicates the detailed documentation maintained in docs/MCP.md. Replace the endpoint, tools, configuration, environment-variable, and rate-limit details with a brief MCP server summary and a link to docs/MCP.md as the single source of truth.web/src/lib/community.ts (1)
105-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exposing a reset for
hasLoggedCommunityFetchFailure.
web/src/lib/external-icons.tsandweb/src/lib/api.tsboth expose test helpers that reset their one-time log flags. This module keeps the flag permanently, so tests that assert the warning cannot run in isolation after another test triggers it.♻️ Proposed helper
+export function clearCommunityFetchFailureStateForTests(): void { + hasLoggedCommunityFetchFailure = false +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/community.ts` around lines 105 - 112, Expose a test helper near logCommunityFetchFailure that resets hasLoggedCommunityFetchFailure to false, matching the reset helpers in external-icons.ts and api.ts. Keep the one-time warning behavior unchanged in production and make the helper available to tests.web/src/lib/icons/search.ts (2)
7-25: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReduce the cost of
levenshteinDistance.The function allocates a full
(b.length + 1) × (a.length + 1)matrix.fuzzySearchcallscalculateStringSimilarityfor the name, every alias, every category, and every query-word/text-word pair.filterAndSortIconsruns that for every icon, andweb/src/components/command-menu.tsxruns it on every keystroke. For a few thousand icons this allocates thousands of arrays per query.Use a two-row rolling buffer, and return early when the length difference already exceeds any useful similarity.
♻️ Proposed refactor
export function levenshteinDistance(a: string, b: string): number { - const matrix: number[][] = [] - - for (let i = 0; i <= b.length; i++) { - matrix[i] = [i] - } - for (let j = 0; j <= a.length; j++) { - matrix[0][j] = j - } - - for (let i = 1; i <= b.length; i++) { - for (let j = 1; j <= a.length; j++) { - const cost = a[j - 1] === b[i - 1] ? 0 : 1 - matrix[i][j] = Math.min(matrix[i - 1][j] + 1, matrix[i][j - 1] + 1, matrix[i - 1][j - 1] + cost) - } - } - - return matrix[b.length][a.length] + if (a === b) return 0 + if (!a.length) return b.length + if (!b.length) return a.length + + let prev = new Array<number>(a.length + 1) + let curr = new Array<number>(a.length + 1) + for (let j = 0; j <= a.length; j++) prev[j] = j + + for (let i = 1; i <= b.length; i++) { + curr[0] = i + for (let j = 1; j <= a.length; j++) { + const cost = a[j - 1] === b[i - 1] ? 0 : 1 + curr[j] = Math.min(prev[j] + 1, curr[j - 1] + 1, prev[j - 1] + cost) + } + const swap = prev + prev = curr + curr = swap + } + + return prev[a.length] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/icons/search.ts` around lines 7 - 25, Optimize levenshteinDistance by replacing the full matrix allocation with two rolling rows that are reused across iterations. Before calculating the rows, return early when the input length difference exceeds the maximum useful similarity threshold expected by calculateStringSimilarity, while preserving the existing distance result for all other inputs.
108-159: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare the scoring weights between
scoreIconandfilterAndSortIcons.
NAME_WEIGHTandALIAS_WEIGHTare declared twice, and the name/alias scoring expression is duplicated.web/src/lib/icons/service.tscallsfilterAndSortIconsfor ranking andscoreIconfor the reported score, so the two formulas must stay in agreement. Extract the weights to module constants and reusescoreIconinside the mapping step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/icons/search.ts` around lines 108 - 159, Update scoreIcon and filterAndSortIcons to share module-level NAME_WEIGHT and ALIAS_WEIGHT constants, removing their local duplicates. In filterAndSortIcons, reuse scoreIcon for the name/alias scoring portion of each mapped icon, while preserving the existing category scoring, penalty, sorting, and ranking behavior so reported scores remain consistent.web/src/lib/icons/urls.ts (1)
18-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
buildIconUrlfor the color variants.Lines 27 and 30 repeat the URL format.
Icon.baseis typed asstring, so a value other thansvg,png, orwebpproduces a path with an unsupported extension. NarrowbasetoIconFormatand delegate tobuildIconUrl.♻️ Proposed refactor
export function buildIconUrls(name: string, icon: Icon) { - const base = typeof icon.base === "string" && !icon.base.startsWith("http") ? icon.base : "svg" + const base: IconFormat = icon.base === "png" || icon.base === "webp" ? icon.base : "svg" const urls: { svg: string; png: string; webp: string; light?: string; dark?: string } = { svg: buildIconUrl(name, "svg", "default"), png: buildIconUrl(name, "png", "default"), webp: buildIconUrl(name, "webp", "default"), } if (icon.colors?.light) { - urls.light = `${BASE_URL}/${base}/${icon.colors.light}.${base}` + urls.light = buildIconUrl(icon.colors.light, base) } if (icon.colors?.dark) { - urls.dark = `${BASE_URL}/${base}/${icon.colors.dark}.${base}` + urls.dark = buildIconUrl(icon.colors.dark, base) } return urls }This also aligns the format selection with
resolveFormatinweb/src/lib/icon-url.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/icons/urls.ts` around lines 18 - 33, Update buildIconUrls to narrow the selected base format to the IconFormat type, allowing only svg, png, or webp values and defaulting unsupported bases to svg. Replace the manually interpolated light and dark variant URLs with buildIconUrl calls using the selected base format and each color variant, preserving the existing default URLs.web/src/lib/icons/__tests__/search.test.ts (1)
56-72: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for long text with a very short query.
The current cases assert only
> 0or=== 0. They do not bound the upper value, so the unbounded return value ofcontainsCharsInOrderflagged inweb/src/lib/icons/search.tspasses unnoticed. Add an upper-bound assertion.💚 Proposed test
it("returns 0 when chars are out of order", () => { expect(containsCharsInOrder("plex", "xlp")).toBe(0) }) + + it("never exceeds 1 for long text with a short query", () => { + expect(containsCharsInOrder("plex media server for home", "p")).toBeLessThanOrEqual(1) + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/icons/__tests__/search.test.ts` around lines 56 - 72, Extend the containsCharsInOrder test coverage with a long-text, very-short-query case and assert that the returned score stays within its intended upper bound, not merely greater than zero. Add the regression assertion in the existing containsCharsInOrder describe block while preserving the current empty-input and ordering cases.web/src/mcp/__tests__/handler.test.ts (1)
9-14: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMock does not catch the
createMcpHandlerargument-arity issue.The mock only reads the first parameter and ignores the rest, so it cannot detect whether the call site passes two or three arguments. See the related comment on
handler.tsaboutmcp-handler2.x's argument signature; a stricter mock that asserts onarguments.lengthor inspects the second argument's shape would have caught that theverboseLogsoption is passed in an argument position the real library ignores.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/mcp/__tests__/handler.test.ts` around lines 9 - 14, The createMcpHandler mock in the test setup is too permissive and cannot detect incorrect argument placement. Update the mock to validate the invocation arity or inspect the second argument’s expected options shape, so tests fail when verboseLogs is passed in an unsupported third argument position while preserving the existing initialization behavior.web/src/app/api/mcp/route.ts (1)
26-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the repeated, inconsistent security headers.
The request-limit 429 (Lines 29-33) sets
Retry-After,Cache-Control,X-Content-Type-Options, andX-Frame-Options. The tool-limit 429 (Lines 49-51) sets onlyRetry-AfterandCache-Control, omitting the other two. The success path (Lines 58-60) repeats the sameCache-Control/X-Content-Type-Options/X-Frame-Optionstriplet a third time.Extract a shared headers object (or a small helper) for the common security headers, and apply it consistently on both 429 responses and the success response.
♻️ Proposed fix
+const SECURITY_HEADERS = { + "Cache-Control": "no-store", + "X-Content-Type-Options": "nosniff", + "X-Frame-Options": "DENY", +} as const + async function guardedHandler(req: NextRequest) { const ip = getClientIp(req.headers) const requestLimit = checkRateLimit(ip, "request") if (!requestLimit.allowed) { return new NextResponse("Too Many Requests", { status: 429, headers: { "Retry-After": String(requestLimit.retryAfter ?? 60), - "Cache-Control": "no-store", - "X-Content-Type-Options": "nosniff", - "X-Frame-Options": "DENY", + ...SECURITY_HEADERS, }, }) } ... if (!toolLimit.allowed) { return new NextResponse("Too Many Requests", { status: 429, headers: { "Retry-After": String(toolLimit.retryAfter ?? 60), - "Cache-Control": "no-store", + ...SECURITY_HEADERS, }, }) } ... const response = await mcpHandler(req) - response.headers.set("Cache-Control", "no-store") - response.headers.set("X-Content-Type-Options", "nosniff") - response.headers.set("X-Frame-Options", "DENY") + for (const [key, value] of Object.entries(SECURITY_HEADERS)) { + response.headers.set(key, value) + } return response }Also applies to: 46-53, 58-60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/app/api/mcp/route.ts` around lines 26 - 34, Consolidate the duplicated common security headers in the MCP route by introducing a shared headers object or helper for Cache-Control, X-Content-Type-Options, and X-Frame-Options. Reuse it in the request-limit 429 response, the tool-limit 429 response, and the success response, while preserving each response’s Retry-After header.web/src/mcp/__tests__/route.test.ts (1)
34-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a test for the missing/understated
content-lengthbypass.This test only verifies that an overstated
content-lengthheader triggers 413 for a small body. It does not verify the more important direction: a large body sent without acontent-lengthheader (or with an understated one) is still rejected. Add that case once the underlying check inroute.tsis fixed to measure actual body size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/mcp/__tests__/route.test.ts` around lines 34 - 43, Extend the POST body-size tests around the existing “returns 413 for oversized POST bodies” case, and update the route’s body validation to measure actual body bytes rather than trusting content-length alone. Add coverage for an oversized body with no content-length and with an understated content-length, asserting both return 413.web/src/lib/icons/__tests__/validate.test.ts (1)
44-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for invalid
namethrough the schema, not just throughassertIconNamedirectly.The suite tests
assertIconNamethrowing directly (Lines 19-26), but never callsgetIconSchema.parse({ name: <invalid> })orgetIconUrlSchema.parse({ name: <invalid> }). Given the transform-throw behavior noted invalidate.ts, add a test asserting what error type these schemas actually throw for an invalid name, so a future regression (or fix) is caught here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/icons/__tests__/validate.test.ts` around lines 44 - 58, Extend the existing getIconSchema and/or getIconUrlSchema tests to parse an invalid icon name through the schema and assert the actual error type produced by the transform. Use the existing invalid-name fixture or assertion conventions from the assertIconName tests, and cover the schema path rather than only calling assertIconName directly.web/src/components/icon-details.tsx (1)
718-763: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize
getAvailableSvgVariantssogetSvgUrl/svgUrlmemoization is effective.
getAvailableSvgVariants()is called directly at Line 718 and returns a new array on every render.getSvgUrl'suseCallback(Lines 720-761) depends on this unstableavailableVariantsreference, so the callback is recreated on every render regardless of theuseCallbackwrapper.svgUrl'suseMemo(Line 763) depends ongetSvgUrl, so it also recomputes every render. The same unstable reference feeds theuseEffectat Lines 765-769, causing its dependency check to run on every render as well.Wrap
getAvailableSvgVariantsinuseMemoso the derived array is stable across renders that do not change its inputs. This makes theuseCallback/useMemoadditions provide their intended benefit.♻️ Proposed fix to stabilize `availableVariants`
- const availableVariants = getAvailableSvgVariants() + const availableVariants = useMemo( + () => getAvailableSvgVariants(), + [isExternalIcon, externalIcon, isCommunityIcon, assetUrls, mainIconUrl, iconData, icon], + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/icon-details.tsx` around lines 718 - 763, Memoize the availableVariants result from getAvailableSvgVariants using useMemo, with dependencies covering the inputs that determine the available variants. Keep getSvgUrl and svgUrl using this stable derived array so their existing memoization remains effective.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/app/api/mcp/route.ts`:
- Around line 37-42: Replace the content-length-only guard in the POST branch of
the MCP route with a bounded body-reading path that counts actual bytes and
aborts once MAX_REQUEST_BODY_BYTES is exceeded, including chunked requests
without a Content-Length header. Ensure the validated body is then reused by
req.clone().json() and mcpHandler(req) without allowing the original unbounded
stream to be consumed.
In `@web/src/components/unoptimized-image.tsx`:
- Around line 20-21: Update the sizeStyle assignment in UnoptimizedImage so
usesFlexibleSizing(className) returns an empty style object rather than inline
width:auto and height:auto; retain the existing width and height values for
non-flexible sizing.
In `@web/src/instrumentation.ts`:
- Around line 1-9: Move the OpenTelemetry imports and setup currently in
instrumentation.ts into a dedicated Node-only module, then update register to
dynamically await-import that module only within the NEXT_RUNTIME === "nodejs"
branch. Keep Edge execution free of any OpenTelemetry loading while preserving
the existing Node setup behavior.
In `@web/src/lib/api.ts`:
- Around line 161-169: Update getAuthorData so it skips authorDataCache.set when
resolveAuthorData returns the UNKNOWN_AUTHOR fallback with login "unknown" and
no author metadata fallback was applied; continue caching valid resolved author
data and preserving the existing cache lookup behavior.
- Around line 105-126: Update fetchGitHubAuthorData to build request headers
without an Authorization entry when GITHUB_TOKEN is unset, while preserving
Bearer authentication when a token exists. Add an AbortSignal.timeout to the
fetch options using the timeout pattern already established by the remote icon
service, so the request terminates rather than hanging.
In `@web/src/lib/external-icons.ts`:
- Around line 101-121: Move the try/catch from the unstable_cache callback in
getCachedExternalIcons to fetchExternalIconsWithTTL, allowing
fetchAllExternalIcons failures to propagate without caching an empty fallback.
In fetchExternalIconsWithTTL, return the empty fallback on failure but only
update _memCache when the fetched data is non-empty; preserve existing TTL
behavior for valid cached results.
In `@web/src/lib/icons/__tests__/service.test.ts`:
- Around line 158-163: The vi.unstubAllEnvs() call in the "rejects local
metadata path in production" test only executes if the assertion passes, causing
NODE_ENV to remain as production in subsequent tests when the assertion fails.
Move vi.unstubAllEnvs() from the end of the test case to the existing afterEach
block to guarantee cleanup runs regardless of test outcome.
In `@web/src/lib/icons/rate-limit.ts`:
- Around line 10-11: Update the module-level buckets Map and its rate-limit
update logic to evict expired entries during periodic or write-time cleanup,
ensuring stale IP+scope keys cannot accumulate indefinitely. Preserve existing
rate-limit behavior for active buckets, and use the existing resetAt timestamps
when determining expiration.
In `@web/src/lib/icons/search.ts`:
- Around line 36-58: Update containsCharsInOrder so its successful-match score
is always normalized to the 0–1 range, preventing long strings with early
matches from exceeding 1 and distorting fuzzySearch ranking. Preserve the
existing zero results for empty or unmatched inputs, and update the affected
expectations in the search tests to reflect the normalized scores.
In `@web/src/lib/icons/service.ts`:
- Around line 27-49: Update fetchMetadataFromRemote so the ETag is stored with
the cached IconFile data, keeping the validator and response body synchronized.
When a 304 response has no cached body available, retry the metadata request
without the If-None-Match header, then return the fetched data and refreshed
cache entry; preserve the existing cached-data path for valid 304 responses.
- Around line 68-85: Update MetadataCacheState and getAllIcons so the global
cache stores the load timestamp alongside data and etag. Before returning
globalThis.__dashboardIconsMetadata.data, compare its age with
CACHE_TTL_SECONDS; reuse it only while fresh, otherwise call getCachedMetadata
and replace the state with the refreshed data, etag, and current timestamp.
- Around line 98-114: Update searchIcons and the filterAndSortIcons usage so
total reflects all icons matching the trimmed query and category before applying
limit, while results remain limited and mapped as currently. Return the
pre-truncation match count instead of results.length, reusing the existing
filtering and sorting behavior.
In `@web/src/lib/icons/validate.ts`:
- Around line 18-42: Update iconNameField and the schemas that use it, including
getIconSchema and getIconUrlSchema, to validate names without allowing
assertIconName() exceptions to escape from a Zod transform. Use superRefine with
ctx.addIssue or an equivalent non-throwing Zod validation path, while preserving
the existing trimming and icon-name validation rules and structured validation
failures.
In `@web/src/mcp/handler.ts`:
- Around line 5-16: Update createDashboardIconsMcpHandler so createMcpHandler
receives a single options object containing both serverInfo and verboseLogs;
remove the separate third argument to ensure MCP_VERBOSE_LOGS is honored by the
two-argument API.
In `@web/src/mcp/tools.ts`:
- Around line 5-61: Update the not-found response branches in
registerDashboardIconsTools for get_icon and get_icon_url to include isError:
true on the returned MCP tool result, while preserving their existing JSON error
payloads and successful responses.
---
Outside diff comments:
In `@web/src/components/submissions-data-table.tsx`:
- Around line 633-650: Restructure the mobile submission row around the button
with onClick={() => setMobileDetailSubmission(submission)} so the selectable
Checkbox is rendered outside that button as a sibling control. Preserve the
existing selection state, toggleMobileSelection behavior, styling, and
detail-button behavior while ensuring the checkbox and detail action are
separate interactive elements.
---
Nitpick comments:
In `@web/README.md`:
- Around line 53-110: The MCP section in web/README.md duplicates the detailed
documentation maintained in docs/MCP.md. Replace the endpoint, tools,
configuration, environment-variable, and rate-limit details with a brief MCP
server summary and a link to docs/MCP.md as the single source of truth.
In `@web/src/app/api/mcp/route.ts`:
- Around line 26-34: Consolidate the duplicated common security headers in the
MCP route by introducing a shared headers object or helper for Cache-Control,
X-Content-Type-Options, and X-Frame-Options. Reuse it in the request-limit 429
response, the tool-limit 429 response, and the success response, while
preserving each response’s Retry-After header.
In `@web/src/components/icon-details.tsx`:
- Around line 718-763: Memoize the availableVariants result from
getAvailableSvgVariants using useMemo, with dependencies covering the inputs
that determine the available variants. Keep getSvgUrl and svgUrl using this
stable derived array so their existing memoization remains effective.
In `@web/src/lib/community.ts`:
- Around line 105-112: Expose a test helper near logCommunityFetchFailure that
resets hasLoggedCommunityFetchFailure to false, matching the reset helpers in
external-icons.ts and api.ts. Keep the one-time warning behavior unchanged in
production and make the helper available to tests.
In `@web/src/lib/icons/__tests__/search.test.ts`:
- Around line 56-72: Extend the containsCharsInOrder test coverage with a
long-text, very-short-query case and assert that the returned score stays within
its intended upper bound, not merely greater than zero. Add the regression
assertion in the existing containsCharsInOrder describe block while preserving
the current empty-input and ordering cases.
In `@web/src/lib/icons/__tests__/validate.test.ts`:
- Around line 44-58: Extend the existing getIconSchema and/or getIconUrlSchema
tests to parse an invalid icon name through the schema and assert the actual
error type produced by the transform. Use the existing invalid-name fixture or
assertion conventions from the assertIconName tests, and cover the schema path
rather than only calling assertIconName directly.
In `@web/src/lib/icons/search.ts`:
- Around line 7-25: Optimize levenshteinDistance by replacing the full matrix
allocation with two rolling rows that are reused across iterations. Before
calculating the rows, return early when the input length difference exceeds the
maximum useful similarity threshold expected by calculateStringSimilarity, while
preserving the existing distance result for all other inputs.
- Around line 108-159: Update scoreIcon and filterAndSortIcons to share
module-level NAME_WEIGHT and ALIAS_WEIGHT constants, removing their local
duplicates. In filterAndSortIcons, reuse scoreIcon for the name/alias scoring
portion of each mapped icon, while preserving the existing category scoring,
penalty, sorting, and ranking behavior so reported scores remain consistent.
In `@web/src/lib/icons/urls.ts`:
- Around line 18-33: Update buildIconUrls to narrow the selected base format to
the IconFormat type, allowing only svg, png, or webp values and defaulting
unsupported bases to svg. Replace the manually interpolated light and dark
variant URLs with buildIconUrl calls using the selected base format and each
color variant, preserving the existing default URLs.
In `@web/src/mcp/__tests__/handler.test.ts`:
- Around line 9-14: The createMcpHandler mock in the test setup is too
permissive and cannot detect incorrect argument placement. Update the mock to
validate the invocation arity or inspect the second argument’s expected options
shape, so tests fail when verboseLogs is passed in an unsupported third argument
position while preserving the existing initialization behavior.
In `@web/src/mcp/__tests__/route.test.ts`:
- Around line 34-43: Extend the POST body-size tests around the existing
“returns 413 for oversized POST bodies” case, and update the route’s body
validation to measure actual body bytes rather than trusting content-length
alone. Add coverage for an oversized body with no content-length and with an
understated content-length, asserting both return 413.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 703227ac-11d7-4c03-b7bb-3bca5253801d
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (71)
README.mdweb/Dockerfileweb/README.mdweb/docs/MCP.mdweb/next.config.tsweb/package.jsonweb/src/app/api/icons/search/route.tsweb/src/app/api/mcp/route.tsweb/src/app/community/[icon]/page.tsxweb/src/app/dashboard/page.tsxweb/src/app/global-error.tsxweb/src/app/icons/[icon]/page.tsxweb/src/app/icons/external/[slug]/page.tsxweb/src/app/not-found.tsxweb/src/app/og/__tests__/external-route.test.tsweb/src/components/add-mcp-button.tsxweb/src/components/add-to-search-bar-button.tsxweb/src/components/command-menu.tsxweb/src/components/editable-icon-details.tsxweb/src/components/hero.tsxweb/src/components/icon-card.tsxweb/src/components/icon-details.tsxweb/src/components/icon-grid.tsxweb/src/components/icon-search.tsxweb/src/components/icon-submission-form.tsxweb/src/components/magicui/magic-card-pointer.tsxweb/src/components/magicui/magic-card.tsxweb/src/components/mcp-highlighted-code.tsxweb/src/components/mcp-setup-dialog-content.tsxweb/src/components/recently-added-icons.tsxweb/src/components/submission-details.tsxweb/src/components/submissions-data-table.tsxweb/src/components/ui/button.tsxweb/src/components/unoptimized-image.tsxweb/src/components/user-button.tsxweb/src/hooks/use-icon-submission-form.tsweb/src/hooks/use-submissions.tsweb/src/instrumentation.tsweb/src/lib/__tests__/api.test.tsweb/src/lib/__tests__/icon-url.test.tsweb/src/lib/__tests__/mcp-setup-config.test.tsweb/src/lib/api.tsweb/src/lib/community.tsweb/src/lib/external-icon-urls.tsweb/src/lib/external-icons.tsweb/src/lib/icon-url.tsweb/src/lib/icons/__tests__/rate-limit.test.tsweb/src/lib/icons/__tests__/search.test.tsweb/src/lib/icons/__tests__/service.test.tsweb/src/lib/icons/__tests__/urls.test.tsweb/src/lib/icons/__tests__/validate.test.tsweb/src/lib/icons/rate-limit.tsweb/src/lib/icons/search.tsweb/src/lib/icons/service.tsweb/src/lib/icons/types.tsweb/src/lib/icons/urls.tsweb/src/lib/icons/validate.tsweb/src/lib/mcp-setup-config.tsweb/src/lib/svg-color-utils.tsweb/src/lib/utils.tsweb/src/mcp/__tests__/handler.test.tsweb/src/mcp/__tests__/route.test.tsweb/src/mcp/__tests__/server-info.test.tsweb/src/mcp/__tests__/tools.test.tsweb/src/mcp/handler.tsweb/src/mcp/server-info.tsweb/src/mcp/tools.tsweb/src/proxy.tsweb/tests/mcp-setup.spec.tsweb/vitest.config.tsweb/vitest.setup.ts
| export const createDashboardIconsMcpHandler = () => | ||
| createMcpHandler( | ||
| (server) => { | ||
| registerDashboardIconsTools(server) | ||
| }, | ||
| { | ||
| serverInfo: getDashboardIconsServerInfo(), | ||
| }, | ||
| { | ||
| verboseLogs: process.env.MCP_VERBOSE_LOGS === "true", | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== dependency declarations =="
for f in package.json web/package.json pnpm-lock.yaml package-lock.json yarn.lock; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n '"mcp-handler"|mcp-handler@|/mcp-handler/|version: "2\.0\.1"|version: 2\.0\.1' "$f" -C 3 || true
fi
done
echo "== handler.ts =="
if [ -f web/src/mcp/handler.ts ]; then
cat -n web/src/mcp/handler.ts
fi
echo "== imports for mcp-handler =="
rg -n "createMcpHandler|from ['\"].*mcp-handler|require\\(['\"].*mcp-handler" web -S || true
echo "== package metadata from lock/package if available =="
python3 - <<'PY'
import json, os
for path in ["package.json","web/package.json","pnpm-lock.yaml","package-lock.json","yarn.lock"]:
if not os.path.exists(path): continue
print(f"\n--- {path} ---")
if path.endswith((".json",".yaml")):
print(open(path,encoding='utf-8').read())
PYRepository: homarr-labs/dashboard-icons
Length of output: 10196
🌐 Web query:
mcp-handler createMcpHandler 2.0.1 signature serverInfo verboseLogs
💡 Result:
In mcp-handler version 2.0.1, the createMcpHandler function signature has been simplified to combine server options and configuration [1]. The signature is: createMcpHandler(initialize, options) [1] Where: - initialize: A function that defines the MCP server (e.g., registering tools, resources, and prompts) [1][2]. - options: A single object that consolidates the previous separate server options and the additional configuration keys [1]. This options object replaces the multi-argument structure used in earlier versions. Key properties within this object include: - serverInfo: An object providing server identity details [1][3]. - verboseLogs: A boolean flag used to enable verbose logging [1][4]. - onEvent: A callback for handling server events [1]. As of version 2.0.1, mcp-handler requires MCP SDK v2 packages (@modelcontextprotocol/server ^2.0.0), zod ^4.2.0, and Node.js 20 or later [1][2]. This version also adopts the 2026-07-28 Model Context Protocol specification, native support for request metadata, and simplified routing, while removing support for legacy transport endpoint routing [1][5][2].
Citations:
- 1: https://www.npmjs.com/package/mcp-handler
- 2: https://vercel.com/changelog/latest-mcp-spec-now-supported-in-mcp-handler
- 3: https://github.com/modelcontextprotocol/typescript-sdk/blob/cc4b4161/packages/server/src/server/createMcpHandler.ts
- 4: https://github.com/vercel/mcp-handler/blob/main/docs/ADVANCED.md
- 5: https://github.com/vercel/mcp-handler/blob/main/CHANGELOG.md
Merge verboseLogs into the createMcpHandler options object.
mcp-handler@2.0.1 uses the two-argument createMcpHandler(initialize, options) form, with serverInfo and verboseLogs as options properties. Passing verboseLogs as a third argument makes Node pass the server options as the second argument and ignore the config object, so MCP_VERBOSE_LOGS=true has no effect.
♻️ Proposed fix
export const createDashboardIconsMcpHandler = () =>
createMcpHandler(
(server) => {
registerDashboardIconsTools(server)
},
{
serverInfo: getDashboardIconsServerInfo(),
+ verboseLogs: process.env.MCP_VERBOSE_LOGS === "true",
},
- {
- verboseLogs: process.env.MCP_VERBOSE_LOGS === "true",
- },
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const createDashboardIconsMcpHandler = () => | |
| createMcpHandler( | |
| (server) => { | |
| registerDashboardIconsTools(server) | |
| }, | |
| { | |
| serverInfo: getDashboardIconsServerInfo(), | |
| }, | |
| { | |
| verboseLogs: process.env.MCP_VERBOSE_LOGS === "true", | |
| }, | |
| ) | |
| export const createDashboardIconsMcpHandler = () => | |
| createMcpHandler( | |
| (server) => { | |
| registerDashboardIconsTools(server) | |
| }, | |
| { | |
| serverInfo: getDashboardIconsServerInfo(), | |
| verboseLogs: process.env.MCP_VERBOSE_LOGS === "true", | |
| }, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/mcp/handler.ts` around lines 5 - 16, Update
createDashboardIconsMcpHandler so createMcpHandler receives a single options
object containing both serverInfo and verboseLogs; remove the separate third
argument to ensure MCP_VERBOSE_LOGS is honored by the two-argument API.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/docs/MCP.md`:
- Line 101: Update the runtime exporter in instrumentation initialization to use
NEXT_PUBLIC_POSTHOG_HOST when configuring the PostHog ingestion endpoint, while
preserving the EU endpoint as the default when the variable is unset;
alternatively remove the configuration entry from the MCP documentation if host
customization is not intended.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1976666-6ce3-44e3-b489-7f3f16d1c192
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
web/docs/MCP.mdweb/package.jsonweb/src/app/api/mcp/route.tsweb/src/mcp/__tests__/analytics.test.tsweb/src/mcp/__tests__/handler.test.tsweb/src/mcp/__tests__/route.test.tsweb/src/mcp/analytics.tsweb/src/mcp/handler.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- web/package.json
- web/src/mcp/tests/handler.test.ts
- web/src/mcp/tests/route.test.ts
- web/src/mcp/handler.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/src/lib/icons/__tests__/service.test.ts (1)
209-220: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify that the retry is unconditional.
The test only checks that
fetchruns twice. It passes if the retry still sendsIf-None-Match. Assert that the second request has noIf-None-Matchheader.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/icons/__tests__/service.test.ts` around lines 209 - 220, Update the “retries an unexpected 304 without a cached body” test to inspect the second fetch request and assert it does not include an If-None-Match header, while retaining the existing two-call and metadata assertions.web/src/instrumentation-node.ts (1)
4-4: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a batch processor for production PostHog logs.
SimpleLogRecordProcessorexports each record separately. Request-level logs can therefore create one OTLP request per record. OpenTelemetry warns about this overhead and recommendsBatchLogRecordProcessorfor production use. (open-telemetry.github.io)Proposed change
-import { LoggerProvider, SimpleLogRecordProcessor } from "`@opentelemetry/sdk-logs`" +import { BatchLogRecordProcessor, LoggerProvider } from "`@opentelemetry/sdk-logs`" - processors: [new SimpleLogRecordProcessor({ exporter: logExporter })], + processors: [new BatchLogRecordProcessor({ exporter: logExporter })],Also applies to: 15-17
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/instrumentation-node.ts` at line 4, In the logger setup using LoggerProvider, replace SimpleLogRecordProcessor with BatchLogRecordProcessor so records are buffered and exported in batches for production PostHog logs. Update the import and processor construction while preserving the existing exporter configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web/src/instrumentation-node.ts`:
- Line 4: In the logger setup using LoggerProvider, replace
SimpleLogRecordProcessor with BatchLogRecordProcessor so records are buffered
and exported in batches for production PostHog logs. Update the import and
processor construction while preserving the existing exporter configuration.
In `@web/src/lib/icons/__tests__/service.test.ts`:
- Around line 209-220: Update the “retries an unexpected 304 without a cached
body” test to inspect the second fetch request and assert it does not include an
If-None-Match header, while retaining the existing two-call and metadata
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f3653f2-c10f-4b29-a4b9-e7042077404d
📒 Files selected for processing (24)
web/README.mdweb/src/app/api/mcp/route.tsweb/src/components/icon-details.tsxweb/src/components/submissions-data-table.tsxweb/src/components/unoptimized-image.tsxweb/src/instrumentation-node.tsweb/src/instrumentation.tsweb/src/lib/__tests__/api.test.tsweb/src/lib/api.tsweb/src/lib/community.tsweb/src/lib/external-icons.tsweb/src/lib/icons/__tests__/rate-limit.test.tsweb/src/lib/icons/__tests__/search.test.tsweb/src/lib/icons/__tests__/service.test.tsweb/src/lib/icons/__tests__/validate.test.tsweb/src/lib/icons/rate-limit.tsweb/src/lib/icons/search.tsweb/src/lib/icons/service.tsweb/src/lib/icons/urls.tsweb/src/lib/icons/validate.tsweb/src/mcp/__tests__/handler.test.tsweb/src/mcp/__tests__/route.test.tsweb/src/mcp/__tests__/tools.test.tsweb/src/mcp/tools.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- web/src/lib/icons/tests/validate.test.ts
- web/src/lib/community.ts
- web/src/components/unoptimized-image.tsx
- web/src/mcp/tests/tools.test.ts
- web/src/lib/external-icons.ts
- web/src/components/submissions-data-table.tsx
- web/src/mcp/tests/route.test.ts
- web/src/lib/icons/urls.ts
- web/src/mcp/tools.ts
- web/README.md
- web/src/lib/icons/search.ts
- web/src/lib/icons/validate.ts
- web/src/lib/api.ts
- web/src/lib/icons/service.ts
- web/src/app/api/mcp/route.ts
- web/src/lib/icons/rate-limit.ts
- web/src/mcp/tests/handler.test.ts
- web/src/lib/icons/tests/rate-limit.test.ts
- web/src/lib/icons/tests/search.test.ts
- web/src/components/icon-details.tsx
- web/src/lib/tests/api.test.ts
Merge verboseLogs into the mcp-handler options object, batch PostHog OTLP log exports, and assert unconditional metadata retry without If-None-Match.
2 participants
Summary
This PR adds an HTTP MCP server for Dashboard Icons and ships the full in-app onboarding experience so users can connect Cursor, Claude, VS Code, and other MCP clients without leaving the site.
MCP HTTP server
/api/mcpwith tools:search_icons,get_icon,get_icon_url, andsuggest_iconweb/src/lib/iconsgetDashboardIconsServerInfo()so MCPinitializeresponses include site branding (title,description,websiteUrl) and favicon URLs (favicon-96x96.png,favicon.svg)web/docs/MCP.mdMCP setup UI
AddMcpButtonandAddMcpLinkopen a setup dialog from multiple entry points:/icons)McpSetupDialogContentprovides:McpHighlightedCode)mcp-setup-config.tscentralizes endpoint URLs, JSON/bash snippet builders, client metadata, and PostHogmcp_setup_openedsource tracking (hero,icon_search,icon_details)sm:max-w-3xlfor readable config blocksImage and UI fixes
UnoptimizedImage: applieswidth/height: autoonly whenclassNameuses flexible sizing (max-w-*,max-h-*,w-full,h-full); fixed dimensions otherwise. Fixes Next.js aspect-ratio console warnings and prevents oversized MCP icon renderingnext/imagefillusage with explicitUnoptimizedImagesizing in icon cards, recently added icons, icon details, editable icon details, and submission detailsButton: addedReact.forwardRefso RadixDialogTrigger asChildworks reliably on desktop and mobilenext.config.tsimage remote patterns for jsDelivr dashboard-icons paths and Simple Icons CDNBuild, lint, and platform hardening (earlier commits on this branch)
middleware.ts→proxy.ts)Testing
mcp-setup-config,server-info, and additionalapi.test.tsbranchesweb/tests/mcp-setup.spec.ts): 7 scenarios × 2 projects (chromium + mobile-chrome) = 14 tests/iconsbrowse page,/icons/plexwith contextual example via link and sidebar buttonlocalStorage,scrollIntoViewIfNeeded, serial entry-point testsTest plan
pnpm lintpassespnpm testpasses with 100% Vitest coverage on included filespnpm buildcompletes successfullypnpm exec playwright test tests/mcp-setup.spec.tspasses (chromium + mobile-chrome)web/docs/MCP.mdagainst a running dev server (pnpm dev:webon port 3005)initializeresponse at/api/mcpincludes favicon inserverInfo.iconsNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Bug Fixes
Documentation