diff --git a/apps/frontend/src/app/pages/AnalysisArchitectureRedirect.tsx b/apps/frontend/src/app/pages/AnalysisArchitectureRedirect.tsx new file mode 100644 index 0000000..237eb85 --- /dev/null +++ b/apps/frontend/src/app/pages/AnalysisArchitectureRedirect.tsx @@ -0,0 +1,20 @@ +import { useEffect } from 'react'; +import { useNavigate, useParams } from 'react-router-dom'; +import { useAppStore } from '@/app/store/useAppStore'; + +// A repository-scoped deep link into Architecture (#179): the surface itself +// is only ever mounted at the top-level `/architecture` route and reads the +// globally selected repository, so this selects the requested repository +// first and hands off to that route rather than duplicating the surface. +export function AnalysisArchitectureRedirect() { + const { id } = useParams<{ id: string }>(); + const navigate = useNavigate(); + const setActiveRepositoryId = useAppStore((state) => state.setActiveRepositoryId); + + useEffect(() => { + if (id) setActiveRepositoryId(id); + navigate('/architecture', { replace: true }); + }, [id, navigate, setActiveRepositoryId]); + + return null; +} diff --git a/apps/frontend/src/app/pages/AnalysisPipelinePage.integration.test.tsx b/apps/frontend/src/app/pages/AnalysisPipelinePage.integration.test.tsx new file mode 100644 index 0000000..8df122d --- /dev/null +++ b/apps/frontend/src/app/pages/AnalysisPipelinePage.integration.test.tsx @@ -0,0 +1,150 @@ +import { act, render, screen } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { useAppStore } from '@/app/store/useAppStore'; +import { backendService } from '@/shared/services/backend'; +import { NetworkError } from '@/shared/services/api'; +import type { Repository } from '@/shared/types'; +import { AnalysisPipelinePage } from './AnalysisPipelinePage'; + +// Exercises the real hook wired to the real store, unlike the sibling +// AnalysisPipelinePage.test.tsx which mocks the hook entirely -- #175 was a +// gap between a hook that transitions correctly in isolation and a page that +// never actually saw the transition land. +vi.mock('@/features/repositories/hooks/useRepository', () => ({ + useRepository: () => ({ repositories: useAppStore((state) => state.repositories) }), +})); + +const repository: Repository = { + id: 'repo-1', + name: 'sample', + source: 'github', + size: 10, + fileCount: 1, + status: 'analysing', + analysisStage: null, + analysisProgress: 0, + uploadedAt: '2026-07-22T08:00:00Z', + meta: null, + fileTree: [], +}; + +function renderPage() { + return render( + + + } /> + Repo Detail Page} /> + + , + ); +} + +describe('AnalysisPipelinePage integration with the real polling hook', () => { + beforeEach(() => { + useAppStore.setState({ repositories: [repository], analysisRunning: true }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it('navigates to the analysed repository as soon as the backend reports completed, without a reload', async () => { + vi.useFakeTimers(); + vi.spyOn(backendService, 'startAnalysis').mockResolvedValue({ + repositoryId: repository.id, + status: 'queued', + jobId: 'job-1', + }); + vi.spyOn(backendService, 'fetchAnalysisStatus').mockResolvedValue({ + repositoryId: repository.id, + status: 'completed', + jobId: 'job-1', + stage: 'completed', + progress: 100, + startedAt: '2026-07-22T08:00:01Z', + completedAt: '2026-07-22T08:00:02Z', + error: null, + }); + + // MemoryRouter navigation is a client-side state transition, not a + // browser reload -- this test proves the SPA route swap happens on its + // own, with no manual reload step anywhere in the flow. + renderPage(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + + expect(screen.getByText('Repo Detail Page')).toBeInTheDocument(); + }); + + it('renders the Analysis Failed terminal state when the job fails server-side', async () => { + vi.useFakeTimers(); + vi.spyOn(backendService, 'fetchAnalysisStatus').mockResolvedValue({ + repositoryId: repository.id, + status: 'failed', + jobId: 'job-1', + stage: 'extracting-modules', + progress: 35, + startedAt: '2026-07-22T08:00:01Z', + completedAt: '2026-07-22T08:00:02Z', + error: 'Extraction crashed.', + }); + + renderPage(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + + expect(screen.getByText('Analysis Failed')).toBeInTheDocument(); + expect(screen.getByText('Extraction crashed.')).toBeInTheDocument(); + }); + + it('recovers from a transient poll error into the completed terminal state, without ever showing Analysis Failed', async () => { + vi.useFakeTimers(); + const fetchStatus = vi + .spyOn(backendService, 'fetchAnalysisStatus') + .mockRejectedValueOnce(new NetworkError('/analysis/repo-1/status')) + .mockResolvedValueOnce({ + repositoryId: repository.id, + status: 'running', + jobId: 'job-1', + stage: 'extracting-modules', + progress: 35, + startedAt: '2026-07-22T08:00:01Z', + completedAt: null, + error: null, + }) + .mockResolvedValue({ + repositoryId: repository.id, + status: 'completed', + jobId: 'job-1', + stage: 'completed', + progress: 100, + startedAt: '2026-07-22T08:00:01Z', + completedAt: '2026-07-22T08:00:02Z', + error: null, + }); + + renderPage(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(screen.getByText('Connection lost — retrying…')).toBeInTheDocument(); + expect(screen.queryByText('Analysis Failed')).not.toBeInTheDocument(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(1000); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(1500); + }); + + expect(screen.getByText('Repo Detail Page')).toBeInTheDocument(); + expect(fetchStatus.mock.calls.length).toBeGreaterThanOrEqual(3); + }); +}); diff --git a/apps/frontend/src/app/pages/NotFoundPage.tsx b/apps/frontend/src/app/pages/NotFoundPage.tsx new file mode 100644 index 0000000..2a7c620 --- /dev/null +++ b/apps/frontend/src/app/pages/NotFoundPage.tsx @@ -0,0 +1,20 @@ +import { useNavigate } from 'react-router-dom'; +import { Compass } from 'lucide-react'; +import { EmptyState } from '@/shared/components/ui/EmptyState'; +import { PageHeader } from '@/shared/components/ui/PageHeader'; + +export function NotFoundPage() { + const navigate = useNavigate(); + + return ( +
+ + navigate('/') }} + /> +
+ ); +} diff --git a/apps/frontend/src/app/routes/router.test.tsx b/apps/frontend/src/app/routes/router.test.tsx new file mode 100644 index 0000000..f10f4a4 --- /dev/null +++ b/apps/frontend/src/app/routes/router.test.tsx @@ -0,0 +1,69 @@ +import { render, screen } from '@testing-library/react'; +import { Outlet, RouterProvider } from 'react-router-dom'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { useAppStore } from '@/app/store/useAppStore'; + +// Bypasses auth/repository-fetch plumbing the same way productSurfaces.test.tsx +// and accessibility.test.tsx do, so these tests exercise real route matching +// (#179) without needing a live backend. +vi.mock('./RequireAuth', () => ({ RequireAuth: () => })); +vi.mock('@/shared/components/layout/MainLayout', () => ({ MainLayout: () => })); +vi.mock('@/features/repositories/hooks/useRepository', () => ({ + useRepository: () => ({ + repositories: [], + completedRepositories: [], + activeRepository: null, + loading: false, + error: null, + selectRepository: () => undefined, + removeRepository: async () => undefined, + refresh: () => undefined, + retry: () => undefined, + }), +})); + +import { createAppRouter } from './router'; + +const ROUTE_RENDER_TIMEOUT_MS = 3_000; +let activeRouter: ReturnType | null = null; + +function renderFreshRoute(path: string) { + window.history.replaceState({}, '', path); + activeRouter = createAppRouter(); + return render(); +} + +describe('deep-linked route reachability (#179)', () => { + afterEach(() => { + activeRouter?.dispose(); + activeRouter = null; + window.history.replaceState({}, '', '/'); + useAppStore.setState({ activeRepositoryId: null }); + }); + + it('renders Dashboard directly at /dashboard instead of 404ing', async () => { + renderFreshRoute('/dashboard'); + + expect( + await screen.findByRole('heading', { name: 'Dashboard', level: 1 }, { timeout: ROUTE_RENDER_TIMEOUT_MS }), + ).toBeInTheDocument(); + }); + + it('renders Architecture directly at /analysis/{id}/architecture, selecting that repository', async () => { + renderFreshRoute('/analysis/repo-42/architecture'); + + expect( + await screen.findByRole('heading', { name: 'Architecture', level: 1 }, { timeout: ROUTE_RENDER_TIMEOUT_MS }), + ).toBeInTheDocument(); + expect(useAppStore.getState().activeRepositoryId).toBe('repo-42'); + }); + + it('renders a NotFound page instead of crashing on an unknown route', async () => { + renderFreshRoute('/this-route-does-not-exist'); + + expect( + await screen.findByRole('heading', { name: 'Page not found', level: 1 }, { timeout: ROUTE_RENDER_TIMEOUT_MS }), + ).toBeInTheDocument(); + expect(screen.queryByText(/Unexpected Application Error/i)).not.toBeInTheDocument(); + }); +}); diff --git a/apps/frontend/src/app/routes/router.tsx b/apps/frontend/src/app/routes/router.tsx index 4c1243b..a6450e9 100644 --- a/apps/frontend/src/app/routes/router.tsx +++ b/apps/frontend/src/app/routes/router.tsx @@ -1,4 +1,4 @@ -import { createBrowserRouter } from 'react-router-dom'; +import { createBrowserRouter, Navigate } from 'react-router-dom'; import { MainLayout } from '@/shared/components/layout/MainLayout'; import { RequireAuth } from './RequireAuth'; import { productSurfaceRoutes } from './productSurfaces'; @@ -25,6 +25,12 @@ export function createAppRouter() { { element: , children: [ + { + // Dashboard's registered path is `/` (see productSurfaces); this + // alias keeps the plainer, commonly-guessed URL from 404ing. + path: '/dashboard', + element: , + }, { path: '/repositories/:id', lazy: async () => { @@ -39,7 +45,25 @@ export function createAppRouter() { return { Component: AnalysisPipelinePage }; }, }, + { + path: '/analysis/:id/architecture', + lazy: async () => { + const { AnalysisArchitectureRedirect } = await import('@/app/pages/AnalysisArchitectureRedirect'); + return { Component: AnalysisArchitectureRedirect }; + }, + }, ...productSurfaceRoutes, + { + // Catch-all: an unmatched path previously fell through to react-router's + // default error boundary ("Unexpected Application Error! 404 Not Found") + // instead of a page. This still sits behind RequireAuth, so an + // unauthenticated visitor to a bogus path is redirected to /login first. + path: '*', + lazy: async () => { + const { NotFoundPage } = await import('@/app/pages/NotFoundPage'); + return { Component: NotFoundPage }; + }, + }, ], }, ], diff --git a/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.test.ts b/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.test.ts index 5dc4fc7..e57462a 100644 --- a/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.test.ts +++ b/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.test.ts @@ -382,5 +382,36 @@ describe('useAnalysisPipeline', () => { expect(hook.result.current.error).toBe('Extraction crashed.'); expect(hook.result.current.connectionStatus).toBe('connected'); }); + + it('stops polling once a terminal status is observed instead of scheduling another request', async () => { + vi.useFakeTimers(); + const fetchStatus = vi.spyOn(backendService, 'fetchAnalysisStatus').mockResolvedValue({ + repositoryId: repository.id, + status: 'completed', + jobId: 'job-1', + stage: 'completed', + progress: 100, + startedAt: '2026-07-22T08:00:01Z', + completedAt: '2026-07-22T08:00:02Z', + error: null, + }); + + const hook = renderHook(() => useAnalysisPipeline(repository.id)); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(hook.result.current.jobStatus).toBe('completed'); + const callsAtCompletion = fetchStatus.mock.calls.length; + + // Advancing well past the poll interval must not trigger another + // request -- the loop stops itself on a terminal status rather than + // relying on the surrounding effect to tear it down. + await act(async () => { + await vi.advanceTimersByTimeAsync(1500 * 5); + }); + expect(fetchStatus.mock.calls.length).toBe(callsAtCompletion); + }); + }); }); diff --git a/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.ts b/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.ts index a78c152..1b02f77 100644 --- a/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.ts +++ b/apps/frontend/src/features/analysis/hooks/useAnalysisPipeline.ts @@ -43,6 +43,7 @@ export function useAnalysisPipeline(repositoryId: string | undefined) { const repository = repositories.find((repo) => repo.id === repositoryId) || null; const repositoryStatus = repository?.status; + const repositoryErrorMessage = repository?.errorMessage; useEffect(() => { pollingGenerationRef.current += 1; @@ -99,21 +100,20 @@ export function useAnalysisPipeline(repositoryId: string | undefined) { setRefreshKey((key) => key + 1); }, []); - // Known limitation: repository identity changes on each poll, so the interval is recreated. useEffect(() => { - if (!repositoryId || !repository) { + if (!repositoryId || !repositoryStatus) { setStatus('empty'); return; } - if (repository.status === 'completed') { + if (repositoryStatus === 'completed') { setStatus('success'); return; } - if (repository.status === 'error') { + if (repositoryStatus === 'error') { setStatus('error'); - setError(repository.errorMessage || 'Analysis failed.'); + setError(repositoryErrorMessage || 'Analysis failed.'); return; } @@ -176,6 +176,13 @@ export function useAnalysisPipeline(repositoryId: string | undefined) { // A job the API itself reports as failed is a real, terminal outcome -- // render it immediately, distinct from a dropped connection below. applyJobResponse(response); + + // Stop the loop here instead of relying on the surrounding effect to + // tear it down on the next render: the poll loop owns its own + // lifecycle, so a terminal outcome never schedules another request. + if (response.status === 'completed' || response.status === 'failed' || response.status === 'cancelled') { + return; + } } catch (caught) { if (cancelled) return; @@ -215,7 +222,7 @@ export function useAnalysisPipeline(repositoryId: string | undefined) { applyJobResponse, failAnalysis, refreshKey, - repository, + repositoryErrorMessage, repositoryId, repositoryStatus, jobStatus,