diff --git a/src/App.tsx b/src/App.tsx index 0c85134..9e758e7 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -23,7 +23,6 @@ import { PROVIDER_BY_ID, DEFAULT_PROVIDER_ID, CUSTOM_PROVIDER, - MANIFEST_BASE_URL, type Provider, } from './providers'; import { partitionHeaders, sendRequest, sendRequestStreaming, type SendResult } from './send'; @@ -35,7 +34,7 @@ import { type HistoryEntry, } from './services/history'; import { checkHealth, type HealthStatus } from './services/healthCheck'; -import { normalizeBaseUrl } from './services/baseUrl'; +import { defaultBaseUrl, normalizeBaseUrl } from './services/baseUrl'; import { isExecutable, runUserCode } from './runners'; import { buildMarkdownReport } from './services/gist'; import GistModal from './components/GistModal.jsx'; @@ -110,20 +109,6 @@ function recordFromEntries(entries: HeaderEntry[]): Record { return out; } -function defaultBaseUrl(): string { - // Wingman is a Manifest gateway tester first, so default the Base URL to the - // canonical Manifest Cloud gateway — the user shouldn't have to type it. On - // localhost we instead guess the local gateway port so `npm run dev` targets - // a locally-running Manifest. The health badge surfaces reachability / CORS. - if (typeof window === 'undefined') return MANIFEST_BASE_URL; - const { protocol, hostname, port } = window.location; - if (hostname === 'localhost' || hostname === '127.0.0.1') { - if (port === '3002' || port === '3000') return `${protocol}//${hostname}:3001`; - return `${protocol}//${hostname}:${port || '3001'}`; - } - return MANIFEST_BASE_URL; -} - // An external embed (?baseUrl=…, e.g. the Manifest dashboard drawer) is always // the Custom/Manifest preset. Otherwise restore the last-picked preset. function resolveInitialProvider(baseUrlParam: string | null): string { diff --git a/src/components/Composer.tsx b/src/components/Composer.tsx index 6cb9f6e..6439921 100644 --- a/src/components/Composer.tsx +++ b/src/components/Composer.tsx @@ -152,6 +152,7 @@ const HealthBadge: Component<{ status: HealthStatus }> = (p) => { const isError = () => status().kind === 'failed' || status().kind === 'invalid' || + status().kind === 'not-a-gateway' || status().kind === 'http-error'; return ( @@ -180,6 +181,8 @@ function healthLabel(s: HealthStatus): string { return 'invalid URL'; case 'failed': return s.label; + case 'not-a-gateway': + return 'not a gateway'; case 'http-error': return `HTTP ${s.status}`; default: @@ -200,6 +203,8 @@ function healthTitle(s: HealthStatus): string { return s.message; case 'failed': return s.message; + case 'not-a-gateway': + return s.message; case 'http-error': return `${s.probedUrl} returned ${s.status} ${s.statusText}`; default: diff --git a/src/services/baseUrl.test.ts b/src/services/baseUrl.test.ts index 311cb32..910449f 100644 --- a/src/services/baseUrl.test.ts +++ b/src/services/baseUrl.test.ts @@ -1,5 +1,12 @@ import { describe, expect, it } from 'vitest'; -import { hasValidHostname, isLoopbackHost, isPrivateHost, normalizeBaseUrl } from './baseUrl'; +import { + defaultBaseUrl, + hasValidHostname, + isLoopbackHost, + isPrivateHost, + normalizeBaseUrl, +} from './baseUrl'; +import { MANIFEST_BASE_URL } from '../providers'; const CHAT = '/v1/chat/completions'; const MESSAGES = '/v1/messages'; @@ -135,6 +142,37 @@ describe('hasValidHostname', () => { ); }); +describe('defaultBaseUrl', () => { + const at = (hostname: string, port: string, protocol = 'http:') => + defaultBaseUrl({ protocol, hostname, port }); + + it('points at the Manifest dev gateway from a loopback host', () => { + expect(at('localhost', '3002')).toBe('http://localhost:3001'); + expect(at('127.0.0.1', '3000')).toBe('http://127.0.0.1:3001'); + }); + + // The bug: any port outside 3000/3002 was passed straight through, so Wingman + // defaulted to its *own* origin. Its SPA fallback then answers the health + // probe with 200 HTML and the badge goes green while pointing at nothing. + it.each(['44321', '38173', '41999', ''])('never returns its own origin (port %s)', (port) => { + const result = at('localhost', port); + expect(result).not.toBe(`http://localhost:${port}`); + expect(result).toBe('http://localhost:3001'); + }); + + it('has nothing to guess when Wingman is itself on the gateway port', () => { + expect(at('localhost', '3001')).toBe(MANIFEST_BASE_URL); + }); + + it('uses Manifest Cloud from a public host', () => { + expect(at('wingman.manifest.build', '', 'https:')).toBe(MANIFEST_BASE_URL); + }); + + it('preserves the page protocol on loopback', () => { + expect(at('localhost', '8443', 'https:')).toBe('https://localhost:3001'); + }); +}); + describe('isLoopbackHost', () => { it.each(['localhost', 'LOCALHOST', '127.0.0.1', '::1', 'api.localhost'])('%s is loopback', (h) => expect(isLoopbackHost(h)).toBe(true), diff --git a/src/services/baseUrl.ts b/src/services/baseUrl.ts index 75744c2..de2310e 100644 --- a/src/services/baseUrl.ts +++ b/src/services/baseUrl.ts @@ -8,9 +8,14 @@ // reads like the gateway is broken. Normalising here means the common pastes // all resolve to the same request. +import { MANIFEST_BASE_URL } from '../providers'; + /** Hosts the browser treats as the local machine (loopback address space). */ const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '0.0.0.0', '::1', '[::1]']); +/** Port a `npm run dev` Manifest backend listens on. */ +const MANIFEST_DEV_GATEWAY_PORT = '3001'; + /** Every endpoint path a format may append, longest first so the greedy match wins. */ const FORMAT_PATHS = ['/v1/chat/completions', '/v1/messages', '/v1/responses'] as const; @@ -94,6 +99,28 @@ function stripEndpointPath(pathname: string, formatPath: string): { path: string return { path }; } +/** + * The Base URL to start from when the user hasn't picked one. + * + * On a loopback host, guess the Manifest dev backend on :3001 — Wingman is a + * Manifest gateway tester first, so that's the useful default for `npm run + * dev`. What it must never do is return Wingman's *own* origin, which the + * previous port-passthrough did for any port other than 3000/3002. That was + * worse than an unhelpful default: Wingman's SPA fallback answers the health + * probe with `200 index.html`, so the badge went green while pointing at + * Wingman itself. Serving Wingman on an arbitrary loopback port is now normal + * (the Manifest dashboard's dev drawer proxies it onto one), so the passthrough + * had become the common case rather than the edge. + */ +export function defaultBaseUrl(location?: { protocol: string; hostname: string; port: string }): string { + const loc = location ?? (typeof window === 'undefined' ? null : window.location); + if (!loc) return MANIFEST_BASE_URL; + if (!isLoopbackHost(loc.hostname)) return MANIFEST_BASE_URL; + // Wingman is itself on the gateway's port — there's nothing left to guess. + if (loc.port === MANIFEST_DEV_GATEWAY_PORT) return MANIFEST_BASE_URL; + return `${loc.protocol}//${loc.hostname}:${MANIFEST_DEV_GATEWAY_PORT}`; +} + /** * Normalise a user-typed base URL and resolve the URL a request will be sent * to. Never throws — an unusable input comes back with `valid: false` and a diff --git a/src/services/healthCheck.test.ts b/src/services/healthCheck.test.ts new file mode 100644 index 0000000..85701b7 --- /dev/null +++ b/src/services/healthCheck.test.ts @@ -0,0 +1,95 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { checkHealth } from './healthCheck'; + +const HEALTH_PATH = '/api/v1/health'; +const CHAT = '/v1/chat/completions'; + +function mockResponse( + body: string, + { status = 200, contentType = 'application/json' }: { status?: number; contentType?: string } = {}, +) { + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ + ok: status >= 200 && status < 300, + status, + statusText: status === 200 ? 'OK' : 'Not Found', + headers: new Headers(contentType ? { 'content-type': contentType } : {}), + text: async () => body, + })), + ); +} + +afterEach(() => vi.unstubAllGlobals()); + +describe('checkHealth', () => { + it('reports a JSON health payload as reachable', async () => { + mockResponse('{"status":"healthy","uptime_seconds":42}'); + const result = await checkHealth('http://localhost:3001', HEALTH_PATH, CHAT); + expect(result.kind).toBe('ok'); + }); + + // The false green: anything with a single-page-app fallback answers *every* + // path with 200 text/html, so a base URL pointing at a web server (including + // Wingman itself) used to read as a healthy gateway. + it('rejects an HTML body by content-type', async () => { + mockResponse('Wingman', { + contentType: 'text/html; charset=utf-8', + }); + const result = await checkHealth('http://localhost:44321', HEALTH_PATH, CHAT); + expect(result.kind).toBe('not-a-gateway'); + if (result.kind === 'not-a-gateway') { + expect(result.message).toMatch(/HTML, not a health payload/); + expect(result.message).toMatch(/check the Base URL/); + } + }); + + it('rejects an HTML body even when the content-type lies', async () => { + mockResponse(' ', { contentType: 'application/json' }); + expect((await checkHealth('http://localhost:44321', HEALTH_PATH, CHAT)).kind).toBe( + 'not-a-gateway', + ); + }); + + it('accepts a non-JSON, non-HTML body rather than being over-strict', async () => { + mockResponse('ok', { contentType: 'text/plain' }); + expect((await checkHealth('http://localhost:3001', HEALTH_PATH, CHAT)).kind).toBe('ok'); + }); + + it('probes the origin, and says which URL it probed', async () => { + mockResponse('{"status":"healthy"}'); + const result = await checkHealth('http://localhost:3001/v1', HEALTH_PATH, CHAT); + expect(result.kind).toBe('ok'); + if (result.kind === 'ok') expect(result.probedUrl).toBe('http://localhost:3001/api/v1/health'); + }); + + it('surfaces a non-2xx as an http error', async () => { + mockResponse('{"message":"Not Found"}', { status: 404 }); + const result = await checkHealth('http://localhost:3001', HEALTH_PATH, CHAT); + expect(result.kind).toBe('http-error'); + }); + + it('rejects an unusable base URL before fetching', async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal('fetch', fetchSpy); + const result = await checkHealth('not a url at all', HEALTH_PATH, CHAT); + expect(result.kind).toBe('invalid'); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('is idle for an empty base URL', async () => { + expect((await checkHealth(' ', HEALTH_PATH, CHAT)).kind).toBe('idle'); + }); + + it('classifies a rejected fetch', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + throw new TypeError('Failed to fetch'); + }), + ); + const result = await checkHealth('http://localhost:59999', HEALTH_PATH, CHAT); + expect(result.kind).toBe('failed'); + if (result.kind === 'failed') expect(result.label).toBe('unreachable'); + }); +}); diff --git a/src/services/healthCheck.ts b/src/services/healthCheck.ts index 406fd26..b35b3bf 100644 --- a/src/services/healthCheck.ts +++ b/src/services/healthCheck.ts @@ -7,8 +7,21 @@ export type HealthStatus = | { kind: 'ok'; latencyMs: number; probedUrl: string } | { kind: 'invalid'; message: string } | { kind: 'failed'; failure: FailureKind; label: string; message: string; probedUrl: string } + | { kind: 'not-a-gateway'; message: string; probedUrl: string } | { kind: 'http-error'; status: number; statusText: string; probedUrl: string }; +/** + * A 200 alone doesn't mean a gateway answered. Anything serving a single-page + * app — Wingman's own dev server, a catch-all reverse proxy, a hosting rewrite + * rule — returns `200 text/html` for *every* path, health endpoint or not. That + * turned a wrong base URL into a green "reachable" badge, which is the same + * false confidence the probe exists to prevent. + */ +function looksLikeHtml(contentType: string | null, body: string): boolean { + if (contentType && /text\/html/i.test(contentType)) return true; + return body.trimStart().startsWith('<'); +} + /** * Probe a gateway's health endpoint to give the connection bar a live badge. * @@ -51,8 +64,25 @@ export async function checkHealth( return { kind: 'failed', failure, label: advice.label, message: advice.detail, probedUrl }; } - if (res.ok) { - return { kind: 'ok', latencyMs: Math.round(performance.now() - started), probedUrl }; + if (!res.ok) { + return { kind: 'http-error', status: res.status, statusText: res.statusText, probedUrl }; + } + + const latencyMs = Math.round(performance.now() - started); + let body = ''; + try { + body = await res.text(); + } catch { + /* an unreadable body is still a 200 from something; fall through */ + } + if (looksLikeHtml(res.headers.get('content-type'), body)) { + return { + kind: 'not-a-gateway', + message: + `${probedUrl} returned HTML, not a health payload. That host is serving a web page ` + + 'for every path, so it is probably not the gateway — check the Base URL.', + probedUrl, + }; } - return { kind: 'http-error', status: res.status, statusText: res.statusText, probedUrl }; + return { kind: 'ok', latencyMs, probedUrl }; }