Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/treat-404-as-offline.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fiberplane/mcp-gateway": patch
---

Treat HTTP 404 as offline in health checks - a 404 without a session ID indicates the MCP endpoint doesn't exist
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,6 @@ jobs:

- name: Build packages
run: bun run build

- name: Run tests
run: bun run test
6 changes: 3 additions & 3 deletions packages/core/src/health.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ describe("Health Check System", () => {
await gateway.close();
});

test("should handle server returning 404 as up", async () => {
test("should handle server returning 404 as down", async () => {
// Create a server that returns 404
const notFoundServer = Bun.serve({
port: 0,
Expand All @@ -380,8 +380,8 @@ describe("Health Check System", () => {

const results = await gateway.health.check();

// 404 means server is responding, so it's "up"
expect(results[0]?.health).toBe("up");
// 404 means MCP endpoint doesn't exist, treat as down
expect(results[0]?.health).toBe("down");

notFoundServer.stop();
await gateway.close();
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/health.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ export async function checkServerHealth(
const responseTimeMs = Date.now() - startTime;
const timestamp = Date.now();

// 2xx, 3xx, 4xx all mean server is responding
// Only 5xx or network errors mean "down"
if (response.status < 500) {
// 2xx, 3xx, and 4xx (except 404) mean server is responding
// 404, 5xx, or network errors mean "offline"
if (response.status < 500 && response.status !== 404) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Test expects 404 to be healthy but code now treats it as offline

The health check now treats HTTP 404 as offline, but an existing test in packages/core/src/health.test.ts (the "should handle server returning 404 as up" test) explicitly expects 404 responses to result in health: "up". The test comment states "404 means server is responding, so it's 'up'" which contradicts the new behavior. The test will fail because checkServerHealth now returns status: "offline" for 404, which maps to "down" in the gateway, but the test asserts expect(results[0]?.health).toBe("up").

Fix in Cursor Fix in Web

return {
status: "online",
responseTimeMs,
Expand Down
12 changes: 4 additions & 8 deletions packages/web/src/components/command-filter-input.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,14 @@
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import "@testing-library/jest-dom";
import {
mockFilterAutocomplete,
mockFilterBadge,
mockUseAvailableFilters,
} from "@/test-utils/mocks";
import { mockUseAvailableFilters } from "@/test-utils/mocks";
import { TestApiProvider } from "@/test-utils/test-providers";
import { CommandFilterInput } from "./command-filter-input";

// Set up mocks
// Set up mocks - only mock API hooks, let real components render
// NOTE: We intentionally don't mock FilterAutocomplete or FilterBadge here
// to avoid polluting other test files (Bun's mock.module is global)
mockUseAvailableFilters();
mockFilterAutocomplete();
mockFilterBadge();

// Helper to render with ApiProvider
const renderWithProvider = (component: React.ReactElement) =>
Expand Down
15 changes: 13 additions & 2 deletions packages/web/src/components/filter-autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe("FilterAutocomplete", () => {
/>,
);

expect(screen.getByTestId("autocomplete-dropdown")).toBeInTheDocument();
expect(screen.getByRole("listbox")).toBeInTheDocument();
});

Expand All @@ -68,7 +69,9 @@ describe("FilterAutocomplete", () => {
/>,
);

expect(screen.queryByRole("listbox")).not.toBeInTheDocument();
expect(
screen.queryByTestId("autocomplete-dropdown"),
).not.toBeInTheDocument();
});

test("does not render when open but no suggestions or content", () => {
Expand All @@ -81,7 +84,9 @@ describe("FilterAutocomplete", () => {
/>,
);

expect(screen.queryByRole("listbox")).not.toBeInTheDocument();
expect(
screen.queryByTestId("autocomplete-dropdown"),
).not.toBeInTheDocument();
});

test("renders when open with error content but no suggestions", () => {
Expand Down Expand Up @@ -378,6 +383,12 @@ describe("FilterAutocomplete", () => {
/>,
);

// Root section has aria-label for assistance
expect(screen.getByTestId("autocomplete-dropdown")).toHaveAttribute(
"aria-label",
"Filter assistance",
);
// Nested listbox has aria-label for suggestions
expect(screen.getByRole("listbox")).toHaveAttribute(
"aria-label",
"Filter suggestions",
Expand Down
1 change: 1 addition & 0 deletions packages/web/src/components/filter-autocomplete.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ export function FilterAutocomplete({
return (
<section
ref={listRef}
data-testid="autocomplete-dropdown"
className={cn(
"absolute z-50 mt-1 w-full",
"rounded-md border border-border bg-popover shadow-lg",
Expand Down
33 changes: 22 additions & 11 deletions packages/web/src/components/filter-bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ import "@testing-library/jest-dom";
import { createFilter } from "@fiberplane/mcp-gateway-types";
import {
mockAddFilterDropdown,
mockFilterBadge,
mockUseAvailableFilters,
} from "@/test-utils/mocks";
import { TestApiProvider } from "@/test-utils/test-providers";
import { FilterBarUI } from "./filter-bar";

// Set up mocks for child components
// Note: CommandFilterInput is NOT mocked here - we use the real component
// but mock its dependencies (useAvailableFilters hooks)
// Note: We only mock API hooks and AddFilterDropdown (which has complex state)
// FilterBadge and CommandFilterInput use the real components
// NOTE: We intentionally don't mock FilterBadge to avoid polluting filter-badge.test.tsx
// (Bun's mock.module is global and persists across test files)
mockUseAvailableFilters();
mockFilterBadge();
mockAddFilterDropdown();

// Helper to render with ApiProvider
Expand Down Expand Up @@ -223,11 +223,16 @@ describe("FilterBarUI", () => {
/>,
);

// Check that both filters are rendered by finding their filter preview elements
const filterBadges = screen.getAllByTestId("filter-preview");
expect(filterBadges).toHaveLength(2);
expect(filterBadges[0]).toHaveAttribute("data-filter-badge", "tokens");
expect(filterBadges[1]).toHaveAttribute("data-filter-badge", "client");
// Check that both filters are rendered by finding their remove buttons
// Real FilterBadge renders aria-label like "Remove filter: Tokens greater than 150"
const removeButtons = screen.getAllByRole("button", {
name: /Remove filter/,
});
expect(removeButtons).toHaveLength(2);

// Verify both filter contents are rendered
expect(screen.getByText("Tokens")).toBeInTheDocument();
expect(screen.getByText("Client")).toBeInTheDocument();
});

test("removes filter when badge remove button clicked", () => {
Expand Down Expand Up @@ -255,7 +260,10 @@ describe("FilterBarUI", () => {
/>,
);

const removeButton = screen.getByText("Remove");
// Real FilterBadge has aria-label on remove button
const removeButton = screen.getByRole("button", {
name: /Remove filter/,
});
fireEvent.click(removeButton);

expect(onRemoveFilter).toHaveBeenCalledWith(filter.id);
Expand Down Expand Up @@ -661,7 +669,10 @@ describe("FilterBarUI", () => {
/>,
);

const removeButton = screen.getByText("Remove");
// Real FilterBadge has aria-label on remove button
const removeButton = screen.getByRole("button", {
name: /Remove filter/,
});
fireEvent.click(removeButton);

// Verify onRemoveFilter was called with correct ID
Expand Down
11 changes: 1 addition & 10 deletions packages/web/src/components/ui/McpServerIcon/notion.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 4 additions & 18 deletions packages/web/src/pages/marketplace-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ function MarketplaceServerCard({
<div className="flex-1 min-w-0">
<h3 className="font-semibold text-base mb-0.5">{server.name}</h3>
<p className="text-xs text-muted-foreground font-mono truncate">
{server.command}
{server.type === "http"
? server.command.replace(/^https?:\/\//, "")
: server.command}
</p>
</div>
</div>
Expand All @@ -180,9 +182,7 @@ function MarketplaceServerCard({
className="inline-flex items-center gap-1 px-2 py-1 rounded-full bg-secondary text-xs text-muted-foreground hover:text-foreground transition-colors"
>
<ArrowUpRight className="h-3 w-3" />
<span className="text-foreground">
{getDomainFromUrl(server.docsUrl)}
</span>
<span className="text-foreground">Docs</span>
</a>
)}
</div>
Expand Down Expand Up @@ -244,17 +244,3 @@ function parseCommand(cmdString: string): { command: string; args: string[] } {

return { command, args };
}

/**
* Extract domain name from URL for display
* Example: "https://linear.app/docs/mcp" -> "linear.app"
*/
function getDomainFromUrl(url: string): string {
try {
const hostname = new URL(url).hostname;
// Remove www. prefix if present
return hostname.replace(/^www\./, "");
} catch {
return url;
}
}
Loading