diff --git a/agents-docs/LESSONS.md b/agents-docs/LESSONS.md index 35467ec..0a196fe 100644 --- a/agents-docs/LESSONS.md +++ b/agents-docs/LESSONS.md @@ -53,12 +53,12 @@ Durable rules for AI agents working on this project. Read this file at session s - **Why:** `router.navigate` to a non-existent path raises `NG04002` and aborts navigation, and stale `startsWith` matches silently break route-derived UI state — neither is caught by the build (string literals) and there was no `servers-rail` spec to catch it. - **Example:** fixed `isOnServers`/`router.navigate(['/servers'])` in `servers-rail.component.{ts,html}`; canonical post-leave/discovery route is `/servers` (`FindServersComponent`), matching `DashboardComponent`'s `router.navigate(['/servers'])`. -### Server discovery (featured/trending) must fan out across all online endpoints like search [server-directory] +### Server discovery must fan out across all endpoints and self-heal on 404 — never hardcode a host capability blocklist [server-directory] -- **Trigger:** the `/servers` (find-servers) page showed no servers by default but found them as soon as the user typed in the search box. Discovery (`getDiscoveryServers`) queried only the *active* endpoint via `getApiBaseUrl()`, and when that endpoint is a discovery-unsupported production host (`signal.toju.app` / `signal-sweden.toju.app` in `DISCOVERY_UNSUPPORTED_HOSTS`) it short-circuited to `[]`; search meanwhile fans out across every online endpoint, so typing surfaced the servers that lived on other endpoints (e.g. localhost). -- **Rule:** make `getFeaturedServers`/`getTrendingServers` fan out across `getSearchableEndpoints()` with `forkJoin` + `deduplicateById` (mirroring all-endpoint search), and apply the `endpointSupportsServerDiscovery` gate *per endpoint* (skip → `[]`) instead of short-circuiting the whole request on the active endpoint. -- **Why:** the empty-query find-servers view renders discovery sections, not search results, so any divergence between discovery's endpoint set and search's endpoint set makes the default view look broken while search works. -- **Example:** `getDiscoveryServers` + `fetchDiscoveryFromEndpoint` in `server-directory-api.service.ts`; verified the live server returns 12 featured/12 trending while the active production host is gated out client-side. +- **Trigger:** the dashboard "Popular Servers" and `/servers` discovery view were empty for fresh users until they typed a search. The first fix added a static `DISCOVERY_UNSUPPORTED_HOSTS` blocklist (`signal.toju.app` / `signal-sweden.toju.app`) that short-circuited discovery to `[]`; the production hosts later shipped the `/featured` + `/trending` routes (verified `curl` → 200 with servers), so the stale blocklist kept blocking exactly the default endpoints a fresh account has while ungated search still surfaced them. +- **Rule:** discovery (`getFeaturedServers`/`getTrendingServers`) must fan out across `getSearchableEndpoints()` with `forkJoin` + `deduplicateById` (mirroring all-endpoint search), and detect capability *at runtime* — on a `404` from `/api/servers/{featured,trending}`, fall back per-endpoint to the public `GET /api/servers` listing (`fetchPublicServerListForDiscovery`) instead of returning `[]`. Do not maintain a hardcoded list of hosts that "don't support" a route; it goes stale silently and the build can't catch it. +- **Why:** legacy servers resolve `/featured` as `/servers/:id` and answer 404, so a 404→fallback keeps the default view populated everywhere without a blocklist; the empty-query view renders discovery sections (not search results), so any divergence between discovery and search makes it look broken while search works. +- **Example:** `fetchDiscoveryFromEndpoint` + `fetchPublicServerListForDiscovery` in `server-directory-api.service.ts`; `e2e/tests/servers/server-discovery-default.spec.ts` proves a fresh account sees Popular Servers without searching AND that route-intercepting `/featured`+`/trending` to 404 still populates it via the fallback. ### Server registration needs `ownerPublicKey: oderId || id`, and must not be fire-and-forget [server-directory] [rooms] diff --git a/e2e/tests/servers/server-discovery-default.spec.ts b/e2e/tests/servers/server-discovery-default.spec.ts new file mode 100644 index 0000000..eed61ef --- /dev/null +++ b/e2e/tests/servers/server-discovery-default.spec.ts @@ -0,0 +1,98 @@ +import { test, expect } from '../../fixtures/multi-client'; +import { type Client } from '../../fixtures/multi-client'; +import { RegisterPage } from '../../pages/register.page'; +import { ServerSearchPage } from '../../pages/server-search.page'; +import { dashboardSearchInput, expectDashboardReady } from '../../helpers/dashboard'; +import { MULTI_DEVICE_PASSWORD, uniqueMultiDeviceName } from '../../helpers/multi-device-session'; + +/** + * Regression coverage for: "Fresh users have the server list in dashboard + * completely empty until anything searched." + * + * The directory exposes a curated discovery view (featured/trending) that must + * populate the dashboard "Popular Servers" panel and the /servers page without + * the user typing a search query. A stale client-side host blocklist used to + * short-circuit discovery to [] for the default production endpoints, so servers + * only appeared once a search ran. These tests prove the default view is + * populated, and that discovery self-heals when an endpoint lacks the + * featured/trending routes (older signal servers answer them with 404). + */ +async function createPublicServer(client: Client, username: string, serverName: string): Promise { + const register = new RegisterPage(client.page); + + await register.goto(); + await register.register(username, 'Discovery Host', MULTI_DEVICE_PASSWORD); + await expect(client.page).toHaveURL(/\/dashboard/, { timeout: 15_000 }); + + const search = new ServerSearchPage(client.page); + + await search.createServer(serverName, { description: 'Public discovery server' }); + await expect(client.page).toHaveURL(/\/room\//, { timeout: 15_000 }); +} + +function popularServersPanel(client: Client) { + return client.page.locator('div.rounded-xl', { hasText: 'Popular Servers' }); +} + +test.describe('Server discovery default view', () => { + test.describe.configure({ timeout: 120_000, retries: 1 }); + + test('a fresh account sees public servers in Popular Servers without searching', async ({ createClient }) => { + const suffix = uniqueMultiDeviceName('discovery-default'); + const serverName = `Discovery Default ${suffix}`; + const host = await createClient(); + const visitor = await createClient(); + + await test.step('host registers and publishes a public server', async () => { + await createPublicServer(host, `host_${suffix}`, serverName); + }); + + await test.step('a brand-new account registers', async () => { + const register = new RegisterPage(visitor.page); + + await register.goto(); + await register.register(`visitor_${suffix}`, 'Discovery Visitor', MULTI_DEVICE_PASSWORD); + await expectDashboardReady(visitor.page); + }); + + await test.step('Popular Servers lists the public server with no search query entered', async () => { + await expect(dashboardSearchInput(visitor.page)).toHaveValue(''); + await expect(popularServersPanel(visitor).getByText(serverName)).toBeVisible({ timeout: 30_000 }); + }); + }); + + test('discovery falls back to the public listing when featured/trending routes 404', async ({ createClient }) => { + const suffix = uniqueMultiDeviceName('discovery-fallback'); + const serverName = `Discovery Fallback ${suffix}`; + const host = await createClient(); + const visitor = await createClient(); + + await test.step('host registers and publishes a public server', async () => { + await createPublicServer(host, `host_${suffix}`, serverName); + }); + + await test.step('simulate a legacy signal server without featured/trending routes', async () => { + const notFound = { + status: 404, + contentType: 'application/json', + body: JSON.stringify({ error: 'Server not found', errorCode: 'SERVER_NOT_FOUND' }) + }; + + await visitor.page.route('**/api/servers/featured**', (route) => route.fulfill(notFound)); + await visitor.page.route('**/api/servers/trending**', (route) => route.fulfill(notFound)); + }); + + await test.step('a brand-new account registers against the legacy-style endpoint', async () => { + const register = new RegisterPage(visitor.page); + + await register.goto(); + await register.register(`visitor_${suffix}`, 'Discovery Visitor', MULTI_DEVICE_PASSWORD); + await expectDashboardReady(visitor.page); + }); + + await test.step('Popular Servers still lists the server via the public-listing fallback', async () => { + await expect(dashboardSearchInput(visitor.page)).toHaveValue(''); + await expect(popularServersPanel(visitor).getByText(serverName)).toBeVisible({ timeout: 30_000 }); + }); + }); +}); diff --git a/toju-app/src/app/domains/server-directory/README.md b/toju-app/src/app/domains/server-directory/README.md index 9ae97f3..6928a6d 100644 --- a/toju-app/src/app/domains/server-directory/README.md +++ b/toju-app/src/app/domains/server-directory/README.md @@ -160,7 +160,7 @@ Beyond free-text search, the directory exposes curated discovery lists that powe - `ServerDirectoryFacade.getFeaturedServers()` → `GET /api/servers/featured` - `ServerDirectoryFacade.getTrendingServers()` → `GET /api/servers/trending` -Both pass through `ServerDirectoryService` to `ServerDirectoryApiService.getFeaturedServers()` / `getTrendingServers()`, which share a private `getDiscoveryServers(path)` HTTP helper and normalise results into `ServerInfo[]` exactly like search. Discovery **fans out across every online endpoint** (`getSearchableEndpoints()` + `forkJoin`, deduplicated by ID), mirroring all-endpoint search — querying only the active endpoint made the default `/servers` view appear empty whenever that endpoint was a discovery-unsupported production host (`endpointSupportsServerDiscovery`), even though plenty of servers existed on other online endpoints. Each endpoint that is in `DISCOVERY_UNSUPPORTED_HOSTS` is skipped (returns `[]`) per-endpoint rather than short-circuiting the whole request. The server ranks featured servers (stable curation) and trending servers (recent activity) via `server-ranking.util.ts`; each route caps results at 50 (`parseDiscoveryLimit`). The discovery routes are registered before the parameterised `/:id` route so `featured`/`trending` are not captured as server IDs. +Both pass through `ServerDirectoryService` to `ServerDirectoryApiService.getFeaturedServers()` / `getTrendingServers()`, which share a private `getDiscoveryServers(path)` HTTP helper and normalise results into `ServerInfo[]` exactly like search. Discovery **fans out across every online endpoint** (`getSearchableEndpoints()` + `forkJoin`, deduplicated by ID), mirroring all-endpoint search — querying only the active endpoint made the default `/servers` view appear empty even though plenty of servers existed on other online endpoints. Discovery is **self-healing for legacy servers**: when a `/featured` or `/trending` request returns `404` (older signal servers predate those routes and resolve them as `/servers/:id`), `fetchDiscoveryFromEndpoint` falls back per-endpoint to the public `GET /api/servers` listing (`fetchPublicServerListForDiscovery`) instead of returning `[]`, so a fresh user always sees available servers without searching. The server ranks featured servers (stable curation) and trending servers (recent activity) via `server-ranking.util.ts`; each route caps results at 50 (`parseDiscoveryLimit`). The discovery routes are registered before the parameterised `/:id` route so `featured`/`trending` are not captured as server IDs. `FindServersComponent` (`/servers`) composes these into discovery sections — **Recently active** (the user's saved rooms, capped at 6), **Featured servers**, and **Trending** — and renders them through the reusable `app-server-browser`. `DashboardComponent` (`/dashboard`) uses the same facade methods for its quick search results. diff --git a/toju-app/src/app/domains/server-directory/domain/logic/server-discovery.rules.spec.ts b/toju-app/src/app/domains/server-directory/domain/logic/server-discovery.rules.spec.ts deleted file mode 100644 index 4b623b0..0000000 --- a/toju-app/src/app/domains/server-directory/domain/logic/server-discovery.rules.spec.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { - describe, - expect, - it -} from 'vitest'; - -import { endpointSupportsServerDiscovery } from './server-discovery.rules'; - -describe('server-discovery.rules', () => { - it('skips discovery calls for production signal hosts without featured/trending routes', () => { - expect(endpointSupportsServerDiscovery('https://signal.toju.app')).toBe(false); - expect(endpointSupportsServerDiscovery('https://signal-sweden.toju.app')).toBe(false); - }); - - it('allows discovery on local and custom signal servers', () => { - expect(endpointSupportsServerDiscovery('http://localhost:3001')).toBe(true); - expect(endpointSupportsServerDiscovery('https://signal.example.com')).toBe(true); - }); -}); diff --git a/toju-app/src/app/domains/server-directory/domain/logic/server-discovery.rules.ts b/toju-app/src/app/domains/server-directory/domain/logic/server-discovery.rules.ts deleted file mode 100644 index 95f3e3b..0000000 --- a/toju-app/src/app/domains/server-directory/domain/logic/server-discovery.rules.ts +++ /dev/null @@ -1,13 +0,0 @@ -/** Hostnames known to run older signal servers without featured/trending discovery routes. */ -const DISCOVERY_UNSUPPORTED_HOSTS = new Set(['signal.toju.app', 'signal-sweden.toju.app']); - -/** Returns false when discovery endpoints are known to 404 on the active signal server. */ -export function endpointSupportsServerDiscovery(baseUrl: string): boolean { - try { - const hostname = new URL(baseUrl).hostname; - - return !DISCOVERY_UNSUPPORTED_HOSTS.has(hostname); - } catch { - return true; - } -} diff --git a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts index 225e0e6..40ff889 100644 --- a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts +++ b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts @@ -133,19 +133,19 @@ describe('ServerDirectoryApiService discovery fan-out', () => { expect(calledUrls).toContain('https://other.test/api/servers/featured'); }); - it('does not query discovery-unsupported hosts but still returns the supported endpoints', async () => { + it('queries every online endpoint including production signal hosts', async () => { const { service, get } = createMultiEndpointHarness(endpoints, (url) => { - if (url.startsWith('https://local.test')) - return { servers: [{ id: 's1', name: 'Alpha' }], total: 1 }; + if (url.startsWith('https://signal.toju.app')) + return { servers: [{ id: 'prod-1', name: 'Prod Server' }], total: 1 }; return { servers: [], total: 0 }; }); const result = await firstValueFrom(service.getTrendingServers()); - expect(result.map((server) => server.id)).toEqual(['s1']); + expect(result.map((server) => server.id)).toEqual(['prod-1']); const calledUrls = get.mock.calls.map((call) => call[0] as string); - expect(calledUrls.some((url) => url.includes('signal.toju.app'))).toBe(false); + expect(calledUrls).toContain('https://signal.toju.app/api/servers/trending'); }); it('deduplicates the same server returned by multiple endpoints', async () => { @@ -157,3 +157,83 @@ describe('ServerDirectoryApiService discovery fan-out', () => { expect(result.map((server) => server.id)).toEqual(['dup']); }); }); + +function createObservableHarness( + endpoints: { id: string; name: string; url: string; status: string }[], + getImpl: (url: string) => unknown +) { + const get = vi.fn((url: string) => getImpl(url)); + const http = { get } as unknown as HttpClient; + const endpointState = { + activeServer: () => endpoints[0], + activeServers: () => endpoints, + servers: () => [], + resolveCanonicalEndpoint: (endpoint: unknown) => endpoint ?? null, + findServerByUrl: () => null, + sanitiseUrl: (value: string) => value + } as unknown as ServerEndpointStateService; + const injector = Injector.create({ + providers: [ + ServerDirectoryApiService, + { provide: HttpClient, useValue: http }, + { provide: ServerEndpointStateService, useValue: endpointState } + ] + }); + const service = runInInjectionContext(injector, () => injector.get(ServerDirectoryApiService)); + + return { service, get }; +} + +describe('ServerDirectoryApiService discovery fallback', () => { + const endpoints = [{ id: 'ep-1', name: 'Legacy', url: 'https://local.test', status: 'online' }]; + + it('falls back to the public server listing when a discovery route returns 404', async () => { + const { service, get } = createObservableHarness(endpoints, (url) => { + if (url.includes('/api/servers/featured') || url.includes('/api/servers/trending')) { + return throwError(() => ({ status: 404, error: { errorCode: 'SERVER_NOT_FOUND' } })); + } + + return of({ servers: [{ id: 'legacy-1', name: 'Legacy Server' }], total: 1 }); + }); + const result = await firstValueFrom(service.getFeaturedServers()); + + expect(result.map((server) => server.id)).toEqual(['legacy-1']); + const calledUrls = get.mock.calls.map((call) => call[0] as string); + + expect(calledUrls).toContain('https://local.test/api/servers/featured'); + expect(calledUrls).toContain('https://local.test/api/servers'); + }); + + it('forwards the discovery limit to the public-listing fallback', async () => { + const { service, get } = createObservableHarness(endpoints, (url) => { + if (url.includes('/api/servers/featured')) { + return throwError(() => ({ status: 404 })); + } + + return of({ servers: [], total: 0 }); + }); + + await firstValueFrom(service.getFeaturedServers(7)); + + const fallbackCall = get.mock.calls.find((call) => call[0] === 'https://local.test/api/servers'); + + expect(fallbackCall).toBeDefined(); + expect((fallbackCall?.[1] as { params: HttpParams }).params.get('limit')).toBe('7'); + }); + + it('does not fall back to the public listing on non-404 errors', async () => { + const { service, get } = createObservableHarness(endpoints, (url) => { + if (url.includes('/api/servers/featured')) { + return throwError(() => ({ status: 500 })); + } + + return of({ servers: [{ id: 'unexpected', name: 'Unexpected' }], total: 1 }); + }); + const result = await firstValueFrom(service.getFeaturedServers()); + + expect(result).toEqual([]); + const calledUrls = get.mock.calls.map((call) => call[0] as string); + + expect(calledUrls).not.toContain('https://local.test/api/servers'); + }); +}); diff --git a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts index a8873de..4ed8910 100644 --- a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts +++ b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts @@ -35,7 +35,6 @@ import type { UnbanServerMemberRequest } from '../../domain/models/server-directory.model'; import type { RoomSignalSourceInput } from '../../domain/logic/room-signal-source.logic'; -import { endpointSupportsServerDiscovery } from '../../domain/logic/server-discovery.rules'; interface ServerLookupError { status?: number; @@ -50,6 +49,16 @@ function isServerNotFoundError(error: unknown): boolean { return lookupError?.status === 404 && lookupError.error?.errorCode === 'SERVER_NOT_FOUND'; } +/** + * Older signal servers predate the `/featured` and `/trending` routes, so they + * resolve the request to `/servers/:id` and answer `404`. Treat any 404 from a + * discovery route as "this endpoint has no discovery routes" and fall back to + * the public server listing. + */ +function isDiscoveryRouteUnavailable(error: unknown): boolean { + return (error as ServerLookupError)?.status === 404; +} + @Injectable({ providedIn: 'root' }) export class ServerDirectoryApiService { private readonly http = inject(HttpClient); @@ -302,8 +311,8 @@ export class ServerDirectoryApiService { // Fan discovery out across every online endpoint (mirroring search) so the // default find-servers view isn't empty just because the *active* endpoint - // is a discovery-unsupported production host. Querying only the active - // endpoint made servers invisible until the user typed a search query. + // happens to be one without populated discovery lists. Querying only the + // active endpoint made servers invisible until the user typed a search query. if (onlineEndpoints.length === 0) { return this.fetchDiscoveryFromEndpoint( kind, @@ -326,21 +335,40 @@ export class ServerDirectoryApiService { source: ServerEndpoint | null | undefined, params?: HttpParams ): Observable { - if (!endpointSupportsServerDiscovery(baseUrl)) { - return of([]); - } - return this.http .get<{ servers: ServerInfo[]; total: number }>(`${baseUrl}/api/servers/${kind}`, params ? { params } : {}) .pipe( map((response) => this.normalizeServerList(response, source ?? null)), catchError((error) => { + // Self-heal against legacy signal servers that lack discovery routes: + // fall back to the public listing so the default view is never empty + // just because an endpoint can't rank featured/trending servers. + if (isDiscoveryRouteUnavailable(error)) { + return this.fetchPublicServerListForDiscovery(baseUrl, source, params); + } + console.error(`Failed to get ${kind} servers:`, error); return of([]); }) ); } + private fetchPublicServerListForDiscovery( + baseUrl: string, + source: ServerEndpoint | null | undefined, + params?: HttpParams + ): Observable { + return this.http + .get<{ servers: ServerInfo[]; total: number }>(`${baseUrl}/api/servers`, params ? { params } : {}) + .pipe( + map((response) => this.normalizeServerList(response, source ?? null)), + catchError((error) => { + console.error('Failed to load servers for discovery fallback:', error); + return of([]); + }) + ); + } + private resolveBaseServerUrl(selector?: ServerSourceSelector): string { const resolvedEndpoint = this.resolveEndpoint(selector);