diff --git a/gitnexus-web/e2e/repo-path-identity.spec.ts b/gitnexus-web/e2e/repo-path-identity.spec.ts new file mode 100644 index 000000000..a27928cb2 --- /dev/null +++ b/gitnexus-web/e2e/repo-path-identity.spec.ts @@ -0,0 +1,472 @@ +import { test, expect, type Page } from '@playwright/test'; +import { spawn, type ChildProcess } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +/** + * E2E tests for repo *path* identity with duplicate display names (#2419). + * + * Unlike the sibling specs, this file runs WRITE operations (analyze, + * re-analyze, delete), so it spawns its OWN backend on a dedicated port with + * an isolated GITNEXUS_HOME instead of sharing the suite-wide server: a force + * re-analysis rewrites LadybugDB files under a live server, and doing that on + * the shared instance while parallel workers hold connections has taken the + * whole backend down in CI (every later test in every file died with + * ECONNRESET). Isolation also makes the registry hermetic — exactly the two + * duplicates exist, and nothing here can perturb the other suites. + * + * Two repos with the SAME basename (`pr2419-dupe`) under different parent + * directories are provisioned against that backend via POST /api/analyze. + * Each contains a uniquely named marker file so the tests can assert which + * repo's graph is actually on screen — the whole point of #2419 is that name + * alone cannot distinguish them. + * + * Covers each ambiguity from the issue's "Actual behavior" list, end to end + * through a real browser: + * - duplicate rows render, and the ACTIVE one is identifiable (active-state + * comparison must not use `repo.name === projectName`) + * - switching between duplicates swaps the loaded graph (switching must not + * pass `repo.name` into onSwitchRepo) + * - re-analyze targets the clicked duplicate's exact path, tracks progress + * on that row only, and reconnects to that same duplicate on completion + * - delete removes exactly the chosen duplicate, not its sibling + * - backend HTTP repo resolution treats ?repo= as a path: landing selection + * loads the exact repo, ?repo= survives F5, and a stale path fails closed + * to the repo picker instead of silently retargeting the sibling + */ + +const FRONTEND_URL = process.env.FRONTEND_URL ?? 'http://localhost:5173'; + +// Spec-owned backend (spawned in beforeAll) — deliberately NOT the shared +// suite server; see the header comment. 127.0.0.1 (not localhost) because the +// availability probes here run in Node, whose fetch resolves localhost to an +// address the server may not be bound to. +const BACKEND_PORT = 4799; +const BACKEND_URL = `http://127.0.0.1:${BACKEND_PORT}`; +// Playwright's cwd is gitnexus-web (the config dir). +const CLI_PATH = path.resolve(process.cwd(), '..', 'gitnexus', 'dist', 'cli', 'index.js'); + +const DUPE_NAME = 'pr2419-dupe'; +const READY_TIMEOUT_MS = 45_000; + +interface AnalyzeJobResponse { + jobId: string; +} +interface AnalyzeJobStatus { + status: string; + error?: string; +} +interface RepoListEntry { + name: string; + repoPath?: string; + path?: string; +} + +let tempRoot = ''; +let gitnexusHome = ''; +let server: ChildProcess | undefined; +let serverLog = ''; +let serverExited: number | null | undefined; +/** Duplicate repo paths in registry (= card/switcher-row) order. */ +let dupePaths: string[] = []; + +/** Spawn the spec-owned backend and wait until it serves /api/repos. */ +async function startBackend(): Promise { + gitnexusHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-dupe-home-')); + server = spawn( + process.execPath, + [CLI_PATH, 'serve', '--port', String(BACKEND_PORT), '--host', '127.0.0.1'], + { + env: { ...process.env, GITNEXUS_HOME: gitnexusHome }, + stdio: ['ignore', 'pipe', 'pipe'], + }, + ); + const capture = (chunk: Buffer) => { + serverLog = (serverLog + chunk.toString()).slice(-8_192); + }; + server.stdout?.on('data', capture); + server.stderr?.on('data', capture); + server.on('exit', (code) => { + serverExited = code; + }); + + const deadline = Date.now() + 30_000; + for (;;) { + if (serverExited !== undefined) { + throw new Error(`spec backend exited early (code ${serverExited}):\n${serverLog}`); + } + const ok = await fetch(`${BACKEND_URL}/api/repos`) + .then((r) => r.ok) + .catch(() => false); + if (ok) return; + if (Date.now() > deadline) { + throw new Error(`spec backend did not become ready on ${BACKEND_URL}:\n${serverLog}`); + } + await new Promise((r) => setTimeout(r, 250)); + } +} + +/** + * The marker file proving which duplicate's graph is on screen. Keyed off the + * team-a/team-b path segment (not exact path equality) so macOS + * `/var` → `/private/var` realpath drift can't break the mapping. + */ +function markerFile(repoPath: string): string { + return repoPath.includes(`${path.sep}team-a${path.sep}`) + ? 'team-a-marker.ts' + : 'team-b-marker.ts'; +} + +async function analyzeAndWait(repoPath: string): Promise { + const res = await fetch(`${BACKEND_URL}/api/analyze`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ path: repoPath, force: true }), + }); + if (!res.ok) throw new Error(`POST /api/analyze for ${repoPath} → HTTP ${res.status}`); + const { jobId } = (await res.json()) as AnalyzeJobResponse; + const deadline = Date.now() + 120_000; + for (;;) { + const poll = await fetch(`${BACKEND_URL}/api/analyze/${jobId}`); + const job = (await poll.json()) as AnalyzeJobStatus; + if (job.status === 'complete' || job.status === 'completed') return; + if (job.status === 'failed') throw new Error(`analyze ${repoPath} failed: ${job.error}`); + if (Date.now() > deadline) throw new Error(`analyze ${repoPath} timed out`); + await new Promise((r) => setTimeout(r, 1_000)); + } +} + +async function deleteRepoByPath(repoPath: string): Promise { + await fetch(`${BACKEND_URL}/api/repo?repo=${encodeURIComponent(repoPath)}`, { + method: 'DELETE', + }).catch(() => undefined); +} + +async function listDupes(): Promise { + const res = await fetch(`${BACKEND_URL}/api/repos`); + const repos = (await res.json()) as RepoListEntry[]; + return repos.filter((r) => r.name === DUPE_NAME).map((r) => r.repoPath ?? r.path ?? ''); +} + +test.beforeAll(async () => { + // Backend spawn + two sequential live analyses can exceed the default budget. + test.setTimeout(300_000); + + // Local runs skip gracefully when prerequisites are missing; under E2E=1 + // (CI) a missing prerequisite is an infra failure and must fail loudly. + if (!process.env.E2E) { + const frontendUp = await fetch(FRONTEND_URL) + .then((r) => r.ok) + .catch(() => false); + if (!frontendUp) { + test.skip(true, 'Vite dev server not available'); + return; + } + if (!fs.existsSync(CLI_PATH)) { + test.skip(true, `backend CLI not built (${CLI_PATH})`); + return; + } + } + + await startBackend(); + + // Provision two repos with the SAME basename under different parents. + // The registry (fresh GITNEXUS_HOME) is hermetic by construction. + tempRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gn-dupe-e2e-'))); + const pathA = path.join(tempRoot, 'team-a', DUPE_NAME); + const pathB = path.join(tempRoot, 'team-b', DUPE_NAME); + fs.mkdirSync(pathA, { recursive: true }); + fs.mkdirSync(pathB, { recursive: true }); + fs.writeFileSync( + path.join(pathA, 'team-a-marker.ts'), + 'export function teamAOnly(): string {\n return "team-a";\n}\n', + ); + fs.writeFileSync( + path.join(pathB, 'team-b-marker.ts'), + 'export function teamBOnly(): string {\n return "team-b";\n}\n', + ); + + // Sequential on purpose — concurrent analyses contend for the repo lock. + await analyzeAndWait(pathA); + await analyzeAndWait(pathB); + + dupePaths = await listDupes(); + if (dupePaths.length !== 2) { + throw new Error(`expected 2 registered "${DUPE_NAME}" repos, got ${dupePaths.length}`); + } +}); + +// Every page in this file must talk to the spec-owned backend: both the +// probe-driven landing flow and the ?server= auto-connect read the backend +// URL from useBackend, which honors this supported localStorage override. +test.beforeEach(async ({ page }) => { + await page.addInitScript((backendUrl) => { + window.localStorage.setItem('gitnexus-backend-url', backendUrl); + }, BACKEND_URL); +}); + +test.afterAll(async () => { + if (server && serverExited !== undefined) { + // The backend crashed mid-run — surface its output, which CI otherwise loses. + console.error(`spec backend exited (code ${serverExited}); last output:\n${serverLog}`); + } + if (server && serverExited === undefined) { + // Wait for the process to actually exit before removing its storage, + // otherwise the rm races the server's final writes. + const exited = new Promise((resolve) => server?.once('exit', () => resolve())); + server.kill('SIGTERM'); + await Promise.race([exited, new Promise((r) => setTimeout(r, 5_000))]); + } + try { + if (tempRoot) fs.rmSync(tempRoot, { recursive: true, force: true }); + if (gitnexusHome) fs.rmSync(gitnexusHome, { recursive: true, force: true }); + } catch { + /* best-effort cleanup of temp dirs */ + } +}); + +/** Loads a specific duplicate directly via URL params and waits for Ready. */ +async function connectTo(page: Page, repoPath: string): Promise { + await page.goto( + `/?server=${encodeURIComponent(BACKEND_URL)}&project=${encodeURIComponent(DUPE_NAME)}&repo=${encodeURIComponent(repoPath)}`, + ); + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible({ + timeout: READY_TIMEOUT_MS, + }); +} + +/** The explorer file entry proving which duplicate's graph is on screen. */ +function marker(page: Page, repoPath: string) { + return page.getByText(markerFile(repoPath)).first(); +} + +// ── 1. Landing selection targets the exact path, not the first name match ──── + +test('landing lists both duplicates and selecting the second loads its exact path', async ({ + page, +}) => { + // Plain `/` (no ?server= — that param auto-connects and skips the landing); + // the beforeEach localStorage override points the probe at the spec backend. + await page.goto('/'); + + const dupeCards = page + .locator('[data-testid="landing-repo-card"]') + .filter({ hasText: DUPE_NAME }); + await expect(dupeCards).toHaveCount(2, { timeout: 20_000 }); + + // The second card is the second registry entry — the repo a name-keyed + // lookup would NEVER reach (it always resolves the first match, #2419). + await dupeCards.nth(1).click(); + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible({ + timeout: READY_TIMEOUT_MS, + }); + + const url = new URL(page.url()); + expect(url.searchParams.get('repo')).toBe(dupePaths[1]); + expect(url.searchParams.get('project')).toBe(DUPE_NAME); + + await expect(marker(page, dupePaths[1])).toBeVisible(); + await expect(marker(page, dupePaths[0])).toBeHidden(); +}); + +// ── 2. Header switcher swaps between same-named repos by path ──────────────── + +test('header switcher switches between duplicates and swaps the loaded graph', async ({ page }) => { + await connectTo(page, dupePaths[0]); + await expect(marker(page, dupePaths[0])).toBeVisible(); + + await page.locator('[data-testid="repo-switcher-trigger"]').click(); + + const rows = page.locator('[data-testid="repo-switcher-row"]').filter({ hasText: DUPE_NAME }); + await expect(rows).toHaveCount(2); + + // Issue step 4: "Try to identify the active repository" — exactly one + // duplicate is marked active, and it is the one the URL points at + // (rows render in registry order, so row 0 ↔ dupePaths[0]). + await expect(rows.nth(0)).toHaveAttribute('data-active', 'true'); + await expect(rows.nth(1)).toHaveAttribute('data-active', 'false'); + + const inactiveRow = page + .locator('[data-testid="repo-switcher-row"][data-active="false"]') + .filter({ hasText: DUPE_NAME }); + await inactiveRow.locator('button').first().click(); + + await page.waitForURL((u) => u.searchParams.get('repo') === dupePaths[1], { + timeout: READY_TIMEOUT_MS, + }); + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible({ + timeout: READY_TIMEOUT_MS, + }); + await expect(marker(page, dupePaths[1])).toBeVisible(); + await expect(marker(page, dupePaths[0])).toBeHidden(); + + // Re-open the switcher: the active marker must have followed the switch. + await page.locator('[data-testid="repo-switcher-trigger"]').click(); + await expect(rows.nth(0)).toHaveAttribute('data-active', 'false'); + await expect(rows.nth(1)).toHaveAttribute('data-active', 'true'); +}); + +// ── 3. ?repo= path identity survives reload ────────────────────────────────── + +test('?repo= path identity survives F5 reload', async ({ page }) => { + test.slow(); // two sequential connects (initial + reload) + + await connectTo(page, dupePaths[1]); + + await page.reload(); + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible({ + timeout: READY_TIMEOUT_MS, + }); + + const url = new URL(page.url()); + expect(url.searchParams.get('repo')).toBe(dupePaths[1]); + await expect(marker(page, dupePaths[1])).toBeVisible(); +}); + +// ── 4. Stale ?repo= fails closed instead of retargeting the sibling ────────── + +test('stale ?repo= path falls back to the repo picker, never a same-named sibling', async ({ + page, +}) => { + const stalePath = path.join(tempRoot, 'ghost', DUPE_NAME); + await page.goto( + `/?server=${encodeURIComponent(BACKEND_URL)}&project=${encodeURIComponent(DUPE_NAME)}&repo=${encodeURIComponent(stalePath)}`, + ); + + // Fail-closed: the app must not silently load whichever sibling matches by + // name. The exact recovery surface can be either the error/onboarding path or + // the repo picker while the server probe settles, so assert the identity + // contract instead of overfitting the transient UI phase. + await expect(page.locator('[data-testid="status-ready"]')).toHaveCount(0, { + timeout: 20_000, + }); + await expect(marker(page, dupePaths[0])).toHaveCount(0); + await expect(marker(page, dupePaths[1])).toHaveCount(0); + expect(new URL(page.url()).searchParams.get('repo')).toBe(stalePath); +}); + +// ── 5. Re-analyze targets the exact duplicate, not whatever matches by name ── + +test('re-analyzing a duplicate targets its exact path throughout the flow', async ({ page }) => { + // Live re-index can exceed the default budget. + test.setTimeout(240_000); + + await connectTo(page, dupePaths[0]); + + // Track which repo every subsequent connect-shaped request targets. Attached + // while the app idles on dupePaths[0], so everything recorded from here on + // is driven by the re-analyze flow. + const connectTargets: string[] = []; + page.on('request', (req) => { + const u = new URL(req.url()); + if (u.pathname === '/api/repo' || u.pathname === '/api/graph') { + const target = u.searchParams.get('repo'); + if (target) connectTargets.push(target); + } + }); + + await page.locator('[data-testid="repo-switcher-trigger"]').click(); + const activeRow = page + .locator('[data-testid="repo-switcher-row"][data-active="true"]') + .filter({ hasText: DUPE_NAME }); + const inactiveRow = page + .locator('[data-testid="repo-switcher-row"][data-active="false"]') + .filter({ hasText: DUPE_NAME }); + await expect(inactiveRow).toHaveCount(1); + + // Re-analyze the INACTIVE duplicate — the repo a name-keyed flow would + // confuse with its sibling at every step. + const analyzeRequest = page.waitForRequest( + (req) => req.method() === 'POST' && req.url().includes('/api/analyze'), + ); + await inactiveRow.hover(); + await inactiveRow.locator('[data-testid="repo-switcher-reanalyze"]').click(); + + // The analyze POST must carry the clicked duplicate's path. + const analyzePost = await analyzeRequest; + const body = analyzePost.postDataJSON() as { path?: string }; + expect(body.path).toBe(dupePaths[1]); + const analyzeResponse = await analyzePost.response(); + if (!analyzeResponse) throw new Error('analyze POST received no response'); + if (!analyzeResponse.ok()) { + throw new Error(`analyze POST failed with HTTP ${analyzeResponse.status()}`); + } + + // Progress is tracked per path identity: only the clicked row spins. Under + // name-keyed tracking (`reanalyzing === repo.name`) BOTH rows would spin. + await expect(inactiveRow.locator('.animate-spin')).toHaveCount(1); + await expect(activeRow.locator('.animate-spin')).toHaveCount(0); + + // On completion the app reconnects to the re-analyzed duplicate ITSELF — a + // name-keyed completion would reconnect to the FIRST name match (the + // sibling). Assert the identity of the reconnect at the request level. + // + // Deliberately NOT asserted here: that the reconnect reaches the Ready + // state. A pre-existing storage race (any repo, duplicates or not) can + // leave a freshly re-analyzed database transiently unreadable ("Binder + // exception: Table CodeRelation does not exist") right after completion, + // which would fail this test for reasons unrelated to the #2419 identity + // contract it covers. Tighten to a full Ready assertion once that is fixed. + await expect + .poll(() => connectTargets.filter((t) => t === dupePaths[1]).length, { timeout: 120_000 }) + .toBeGreaterThan(0); + expect(connectTargets).not.toContain(dupePaths[0]); + + // Re-analyze must not duplicate or replace registry entries. + expect((await listDupes()).sort()).toEqual([...dupePaths].sort()); +}); + +// ── 6. Delete removes exactly the chosen duplicate, not its sibling ────────── + +test('deleting one duplicate leaves the same-named sibling registered and loaded', async ({ + page, +}) => { + // Delete retries below may wait out a server-side repo lock. + test.setTimeout(120_000); + + await connectTo(page, dupePaths[0]); + + // Record every DELETE the UI issues — the #2419 contract is that they all + // target exactly the chosen duplicate's path and NEVER the sibling's. + const deleteTargets: string[] = []; + page.on('request', (req) => { + if (req.method() === 'DELETE' && req.url().includes('/api/repo')) { + const target = new URL(req.url()).searchParams.get('repo'); + if (target) deleteTargets.push(target); + } + }); + + await page.locator('[data-testid="repo-switcher-trigger"]').click(); + const inactiveRow = page + .locator('[data-testid="repo-switcher-row"][data-active="false"]') + .filter({ hasText: DUPE_NAME }); + await expect(inactiveRow).toHaveCount(1); + + // Retry the whole click-and-verify block, because two pre-existing server + // races (both unrelated to the #2419 identity contract) can make a single + // click insufficient: a lingering analyze/embed job still holding the repo + // lock 409s the delete, and the registry's validate-prune path can clobber + // a concurrent unregister with its pre-delete snapshot, transiently + // resurrecting the entry after the UI has already dropped the row (in that + // case re-issue the delete by path, off-page, since the row is gone). + await expect(async () => { + if ((await inactiveRow.count()) > 0) { + // Delete icon is revealed on row hover. + await inactiveRow.hover(); + await inactiveRow.locator('[data-testid="repo-switcher-delete"]').click(); + } else if ((await listDupes()).includes(dupePaths[1])) { + await deleteRepoByPath(dupePaths[1]); + } + // Backend: exactly the inactive sibling is gone, the active one remains. + expect(await listDupes()).toEqual([dupePaths[0]]); + }).toPass({ timeout: 90_000, intervals: [2_000] }); + + // Identity: the UI's delete requests all targeted the chosen duplicate. + expect(deleteTargets.length).toBeGreaterThan(0); + expect([...new Set(deleteTargets)]).toEqual([dupePaths[1]]); + + // Frontend: the active repo is untouched — still Ready on the same path. + await expect(page.locator('[data-testid="status-ready"]')).toBeVisible(); + expect(new URL(page.url()).searchParams.get('repo')).toBe(dupePaths[0]); +}); diff --git a/gitnexus-web/src/App.tsx b/gitnexus-web/src/App.tsx index 6141104b5..2f6132c88 100644 --- a/gitnexus-web/src/App.tsx +++ b/gitnexus-web/src/App.tsx @@ -25,6 +25,16 @@ import { parseSkipGraphParam } from './lib/graph-load-decision'; import { formatBackendError } from './i18n/error-messages'; import { useTranslation } from 'react-i18next'; +/** + * Restore-param preference for the auto-connect effect: `repo` carries the + * server-resolved path identity (restores the exact repo even when duplicate + * display names exist, #2419), while older `project`-only URLs degrade to a + * name-based restore. Exported for direct unit testing — no test harness + * renders . + */ +export const pickRestoreRepo = (params: URLSearchParams): string | undefined => + params.get('repo') ?? params.get('project') ?? undefined; + const AppContent = () => { const { t } = useTranslation(['common', 'errors']); const { @@ -66,8 +76,9 @@ const AppContent = () => { result.repoInfo.name || (repoPath || '').replace(/\\/g, '/').split('/').filter(Boolean).pop() || 'server-project'; + const repoIdentity = repoPath || projectName; setProjectName(projectName); - setCurrentRepo(projectName); + setCurrentRepo(repoIdentity); // Build KnowledgeGraph from server data for visualization. In chat-only // mode the graph download was skipped, so the shared builder keeps an @@ -78,9 +89,15 @@ const AppContent = () => { setGraphMode(built.graphMode); setChatOnlyNodeCount(built.graphMode === 'chatOnly' ? built.nodeCount : null); - // Persist the active project in the URL for bookmarkability and F5 refresh resilience + // Persist the active project in the URL for bookmarkability and F5 refresh resilience. + // `repo` carries the server-resolved path identity (never the request-side + // string) so a refresh restores this exact repo even when duplicate display + // names exist (#2419); `project` stays as the readable display name. const urlObj = new URL(window.location.href); urlObj.searchParams.set('project', projectName); + if (repoPath) { + urlObj.searchParams.set('repo', repoPath); + } window.history.replaceState(null, '', urlObj.toString()); // Transition directly to exploring view @@ -90,7 +107,7 @@ const AppContent = () => { // chat-only flag so the agent's prompt matches the loaded/skipped graph (#2178). try { if (getActiveProviderConfig()) { - await initializeAgent(projectName, { chatOnly: result.graphSkipped }); + await initializeAgent(projectName, { chatOnly: result.graphSkipped, repo: repoIdentity }); } startEmbeddingsWithFallback(); } catch (err) { @@ -109,7 +126,11 @@ const AppContent = () => { ], ); - // Auto-connect when ?server or ?project query param is present (bookmarkable shortcut) + // Auto-connect when a ?server, ?repo or ?project query param is present + // (bookmarkable shortcut). A failed ?repo= restore (e.g. the bookmarked path + // was deleted) fails visibly via the error overlay → onboarding — it must + // NOT silently fall back to a name-based connect, which could reconnect a + // same-named sibling repo (#2419). const autoConnectRan = useRef(false); const tRef = useRef(t); useEffect(() => { @@ -120,12 +141,12 @@ const AppContent = () => { if (autoConnectRan.current) return; const params = new URLSearchParams(window.location.search); const serverUrlParam = params.get('server'); - const projectParam = params.get('project'); + const restoreRepoParam = pickRestoreRepo(params); // `?skipGraph=1` forces chat-only, `?skipGraph=0` forces a full graph; // absent → auto-detect by node count. Bookmarkable / survives F5 (#2178). const skipGraphParam = parseSkipGraphParam(params.get('skipGraph')); - if (!serverUrlParam && !projectParam) return; + if (!serverUrlParam && !restoreRepoParam) return; autoConnectRan.current = true; setProgress({ @@ -169,7 +190,7 @@ const AppContent = () => { } }, undefined, - projectParam || undefined, + restoreRepoParam, { awaitAnalysis: true, skipGraph: skipGraphParam }, // hold-queue + chat-only control (#2178) ); }; @@ -282,8 +303,11 @@ const AppContent = () => { setProgress(null); return; } catch (err: unknown) { - if (attempt === 0 && err instanceof BackendError && err.status === 404) { - // Server may still be reinitializing — wait and retry + // Server may still be reinitializing after the worker completed: + // that surfaces as a 404 (repo not registered yet) OR a transient + // 5xx/binder error while the freshly-written DB becomes readable. + // Either way, wait and retry once before giving up. + if (attempt === 0 && err instanceof BackendError) { await new Promise((r) => setTimeout(r, 1500)); continue; } diff --git a/gitnexus-web/src/components/CodeReferencesPanel.tsx b/gitnexus-web/src/components/CodeReferencesPanel.tsx index bd416f9fe..5818614f9 100644 --- a/gitnexus-web/src/components/CodeReferencesPanel.tsx +++ b/gitnexus-web/src/components/CodeReferencesPanel.tsx @@ -57,6 +57,7 @@ export const CodeReferencesPanel = ({ onFocusNode }: CodeReferencesPanelProps) = setSelectedNode, codeReferenceFocus, projectName, + currentRepo, } = useAppState(); const nodeById = useMemo(() => { @@ -226,14 +227,15 @@ export const CodeReferencesPanel = ({ onFocusNode }: CodeReferencesPanelProps) = const isWholeFile = selectedIsFile || startLine === undefined; const options = isWholeFile - ? { repo: projectName } + ? {} : { startLine: Math.max(0, startLine - CONTEXT_LINES), endLine: (endLine ?? startLine) + CONTEXT_LINES, - repo: projectName, }; - readFile(selectedFilePath, { ...options, repo: projectName || undefined }) + // Prefer the repo path identity over the display name — duplicate display + // names would otherwise resolve to the wrong repository's file (#2420). + readFile(selectedFilePath, { ...options, repo: currentRepo || projectName || undefined }) .then((result) => { if (!cancelled) { setFileResult(result); @@ -256,6 +258,7 @@ export const CodeReferencesPanel = ({ onFocusNode }: CodeReferencesPanelProps) = selectedNode?.properties?.endLine, selectedIsFile, projectName, + currentRepo, ]); // Scroll to the selected node's startLine after content loads diff --git a/gitnexus-web/src/components/Header.tsx b/gitnexus-web/src/components/Header.tsx index 5a9ce4277..8e8d41d08 100644 --- a/gitnexus-web/src/components/Header.tsx +++ b/gitnexus-web/src/components/Header.tsx @@ -15,6 +15,7 @@ import { useAppState } from '../hooks/useAppState'; import { deleteRepo, fetchRepos, + repoIdentity, startAnalyze, streamAnalyzeProgress, type BackendRepo, @@ -62,6 +63,7 @@ export const Header = ({ const { t } = useTranslation(['common', 'header', 'errors']); const { projectName, + currentRepo, graph, graphMode, openChatPanel, @@ -74,7 +76,7 @@ export const Header = ({ const [repoSearchQuery, setRepoSearchQuery] = useState(''); const [isRepoDropdownOpen, setIsRepoDropdownOpen] = useState(false); const [showAnalyzer, setShowAnalyzer] = useState(false); - const [reanalyzing, setReanalyzing] = useState(null); // repo name being re-analyzed + const [reanalyzing, setReanalyzing] = useState(null); // repo identity being re-analyzed const [deleteError, setDeleteError] = useState(null); // surfaced when a delete is rejected (e.g. origin-blocked 403) const [reanalyzeProgress, setReanalyzeProgress] = useState(null); const reanalyzeSseRef = useRef(null); @@ -104,6 +106,8 @@ export const Header = ({ return availableRepos.filter((repo) => repo.name.toLowerCase().includes(query)); }, [availableRepos, repoSearchQuery]); + const activeRepoIdentity = currentRepo ?? projectName; + // Handle clicking outside search or repo dropdown to close them useEffect(() => { const handleClickOutside = (e: MouseEvent) => { @@ -187,6 +191,7 @@ export const Header = ({ {projectName && (
) : ( - filteredRepos.map((repo) => ( -
- - {/* Re-analyze */} - - {/* Delete */} - + {/* Re-analyze */} + -
- )) + > + + + {/* Delete */} + + + ); + }) )} @@ -386,7 +416,13 @@ export const Header = ({ {t('header:reanalyzingRepo', { - repoName: reanalyzing, + // `reanalyzing` holds the path identity (#2419) — + // resolve the display name for the label, falling + // back to the path basename. + repoName: + availableRepos.find((r) => repoIdentity(r) === reanalyzing)?.name ?? + reanalyzing.split(/[/\\]/).filter(Boolean).at(-1) ?? + reanalyzing, message: translateProgressMessage(reanalyzeProgress.message, t), })} diff --git a/gitnexus-web/src/components/RepoAnalyzer.tsx b/gitnexus-web/src/components/RepoAnalyzer.tsx index fd08d197d..1aca3a64a 100644 --- a/gitnexus-web/src/components/RepoAnalyzer.tsx +++ b/gitnexus-web/src/components/RepoAnalyzer.tsx @@ -183,7 +183,12 @@ type InternalPhase = 'input' | 'starting' | 'analyzing' | 'done' | 'error'; export interface RepoAnalyzerProps { variant: 'onboarding' | 'sheet'; - onComplete: (repoName: string) => void; + /** + * Receives the repo IDENTITY to reconnect with — the analyzed path when the + * server provides one (`repoPath` on the SSE complete event), otherwise the + * display name. Never rendered; the done screen shows the display name. + */ + onComplete: (repoIdentity: string) => void; onCancel?: () => void; } @@ -360,19 +365,25 @@ export const RepoAnalyzer = ({ variant, onComplete, onCancel }: RepoAnalyzerProp jobId, (p) => setProgress(p), (data) => { - const name = + // Display vs identity split: the done screen renders the display name + // (never an absolute path), while onComplete receives the identity — + // the analyzed path when the server provides it, so the reconnect + // targets the exact repo even when basenames collide. Old servers omit + // repoPath and degrade to today's name behavior. + const displayName = data.repoName ?? (fallbackNameSource ? fallbackNameSource.split(/[/\\]/).filter(Boolean).at(-1) : undefined) ?? t('onboarding:repoAnalyzer.defaultRepoName'); - setCompletedRepoName(name); + const identity = data.repoPath ?? displayName; + setCompletedRepoName(displayName); setGithubToken(''); setPhase('done'); sseControllerRef.current = null; completeTimerRef.current = setTimeout(() => { completeTimerRef.current = null; - onComplete(name); + onComplete(identity); }, 1200); }, (errMsg) => { diff --git a/gitnexus-web/src/components/RepoLanding.tsx b/gitnexus-web/src/components/RepoLanding.tsx index 600312a44..b2b85b259 100644 --- a/gitnexus-web/src/components/RepoLanding.tsx +++ b/gitnexus-web/src/components/RepoLanding.tsx @@ -14,7 +14,7 @@ import { Sparkles, ArrowRight, GitBranch, FileCode, Layers } from '@/lib/lucide-icons'; import { RepoAnalyzer } from './RepoAnalyzer'; -import type { BackendRepo } from '../services/backend-client'; +import { repoIdentity, type BackendRepo } from '../services/backend-client'; import type { TFunction } from 'i18next'; import { useTranslation } from 'react-i18next'; @@ -126,7 +126,11 @@ export const RepoLanding = ({ repos, onSelectRepo, onAnalyzeComplete }: RepoLand {/* Repo list */}
{repos.map((repo) => ( - onSelectRepo(repo.name)} /> + onSelectRepo(repoIdentity(repo))} + /> ))}
diff --git a/gitnexus-web/src/hooks/useAppState.tsx b/gitnexus-web/src/hooks/useAppState.tsx index e3a3daaec..a14be99f2 100644 --- a/gitnexus-web/src/hooks/useAppState.tsx +++ b/gitnexus-web/src/hooks/useAppState.tsx @@ -35,6 +35,9 @@ import { startEmbeddings as backendStartEmbeddings, streamEmbeddingProgress, probeBackend, + // Aliased: switchRepo declares a local `let repoIdentity` that would shadow + // a plain named import of this helper. + repoIdentity as repoIdentityOf, type BackendRepo, type ConnectResult, type JobProgress, @@ -52,6 +55,15 @@ export const shouldAutoStartEmbeddings = (): boolean => { return window.localStorage.getItem(AUTO_START_EMBEDDINGS_STORAGE_KEY) === 'true'; }; +// Resolve a human-readable name for a repo path identity: the registry entry's +// display name first, then the path's basename, then the raw identity. State +// keeps holding the path identity (#2419) — user-facing labels and the agent +// prompt must never show an absolute filesystem path. +const displayNameForIdentity = (repos: BackendRepo[], identity: string): string => + repos.find((r) => repoIdentityOf(r) === identity)?.name ?? + identity.split(/[/\\]/).filter(Boolean).at(-1) ?? + identity; + export type ViewMode = 'onboarding' | 'loading' | 'exploring'; export type RightPanelTab = 'code' | 'chat'; export type EmbeddingStatus = 'idle' | 'loading' | 'embedding' | 'indexing' | 'ready' | 'error'; @@ -162,6 +174,7 @@ interface AppState { // Project info projectName: string; setProjectName: (name: string) => void; + currentRepo: string | undefined; // Multi-repo switching serverBaseUrl: string | null; @@ -169,7 +182,7 @@ interface AppState { availableRepos: BackendRepo[]; setAvailableRepos: (repos: BackendRepo[]) => void; switchRepo: (repoName: string) => Promise; - setCurrentRepo: (repoName: string) => void; + setCurrentRepo: (repoName: string | undefined) => void; /** Download the full graph for the current repo after a chat-only connect (#2178). */ loadGraphAnyway: () => Promise; @@ -204,7 +217,10 @@ interface AppState { // LLM methods refreshLLMSettings: () => void; - initializeAgent: (overrideProjectName?: string, opts?: { chatOnly?: boolean }) => Promise; + initializeAgent: ( + overrideProjectName?: string, + opts?: { chatOnly?: boolean; repo?: string }, + ) => Promise; sendChatMessage: (message: string) => Promise; stopChatResponse: () => void; clearChat: () => void; @@ -346,6 +362,7 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { // Project info const [projectName, setProjectName] = useState(''); + const [currentRepo, setCurrentRepoState] = useState(undefined); // Multi-repo switching const [serverBaseUrl, setServerBaseUrl] = useState(null); @@ -489,8 +506,9 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { // Backend client — direct HTTP calls (no Worker/Comlink) const repoRef = useRef(undefined); - const setCurrentRepo = useCallback((repoName: string) => { + const setCurrentRepo = useCallback((repoName: string | undefined) => { repoRef.current = repoName; + setCurrentRepoState(repoName); }, []); const runQuery = useCallback(async (cypher: string): Promise => { @@ -613,7 +631,10 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { }, [graphMode]); const initializeAgent = useCallback( - async (overrideProjectName?: string, opts?: { chatOnly?: boolean }): Promise => { + async ( + overrideProjectName?: string, + opts?: { chatOnly?: boolean; repo?: string }, + ): Promise => { const config = getActiveProviderConfig(); if (!config) { setAgentError('Please configure an LLM provider in settings'); @@ -632,8 +653,11 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { // Sync repoRef so all agent backend calls target the correct repo. // initializeAgent can be called from App.tsx (handleServerConnect) which // never sets repoRef.current directly — without this, queries default to repo[0]. - if (overrideProjectName) { - repoRef.current = overrideProjectName; + // Only opts.repo may write the identity: overrideProjectName is a display + // name, and a name-only caller must never clobber the path identity with + // an ambiguous name (#2419). + if (opts?.repo) { + setCurrentRepo(opts.repo); } const repo = repoRef.current; @@ -1161,7 +1185,10 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { phase: 'extracting', percent: 0, message: i18n.t('common:progress.switchingRepository'), - detail: i18n.t('common:progress.loadingRepository', { repo: repoName }), + detail: i18n.t('common:progress.loadingRepository', { + // `repoName` is a path identity — show the display name, not the path. + repo: displayNameForIdentity(availableRepos, repoName), + }), }); setViewMode('loading'); setIsAgentReady(false); @@ -1184,7 +1211,10 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { setChatOnlyNodeCount(null); let connectedRepo: BackendRepo | undefined; - let pNameStr = repoName || 'server-project'; + // Bare declarations: both are always assigned on the success path before + // any read, and the catch below returns early (CodeQL alerts 825/826). + let pNameStr: string; + let repoIdentity: string | undefined; let connectedChatOnly = false; try { @@ -1225,12 +1255,13 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { const repoPath = result.repoInfo.repoPath ?? result.repoInfo.path; // Prefer the registry name, then normalize Windows \ and Unix / paths const pName = - repoName || result.repoInfo.name || (repoPath || '').replace(/\\/g, '/').split('/').filter(Boolean).pop() || + repoName || 'server-project'; + repoIdentity = repoName || repoPath || pName; setProjectName(pName); - repoRef.current = pName; + setCurrentRepo(repoIdentity); connectedRepo = result.repoInfo; pNameStr = pName; @@ -1262,11 +1293,19 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { if (pNameStr) { // Persist the selected project in the URL so a refresh re-opens it. + // `repo` carries the server-resolved path identity (never the + // request-side string) so the refresh restores this exact repo even + // when duplicate display names exist (#2419); `project` stays as the + // readable display name. // Drop any `?skipGraph` override: a deliberate repo switch should make a // fresh per-repo decision (auto-detect) on the next refresh rather than // carry the previous repo's forced mode (#2178). const urlObj = new URL(window.location.href); urlObj.searchParams.set('project', pNameStr); + const resolvedRepoPath = connectedRepo?.repoPath ?? connectedRepo?.path; + if (resolvedRepoPath) { + urlObj.searchParams.set('repo', resolvedRepoPath); + } urlObj.searchParams.delete('skipGraph'); window.history.replaceState(null, '', urlObj.toString()); } @@ -1279,7 +1318,7 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { // Re-initialize agent with the new repo's graph context try { if (getActiveProviderConfig()) { - await initializeAgent(pNameStr, { chatOnly: connectedChatOnly }); + await initializeAgent(pNameStr, { chatOnly: connectedChatOnly, repo: repoIdentity }); } setViewMode('exploring'); startEmbeddingsWithFallback(); @@ -1295,6 +1334,7 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { }, [ serverBaseUrl, + availableRepos, setProgress, setViewMode, setProjectName, @@ -1313,6 +1353,7 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { setCodePanelOpen, setCodeReferenceFocus, setChatMessages, + setCurrentRepo, ], ); @@ -1390,7 +1431,14 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { // the chat-only note (#2178, KTD2). Guarded on a configured provider, like // switchRepo; runs inside the mounted/stale guard above. if (getActiveProviderConfig()) { - await initializeAgent(repo, { chatOnly: false }); + // Pass the display name explicitly — initializeAgent's empty-deps + // closure traps `projectName` at its initial '', so relying on the + // state fallback would label the prompt the literal 'project'. The + // path identity travels separately via opts.repo. + await initializeAgent(repo ? displayNameForIdentity(availableRepos, repo) : undefined, { + chatOnly: false, + repo, + }); } } catch (err) { if (!loadGraphMountedRef.current || repoRef.current !== repo) return; @@ -1404,6 +1452,7 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { } }, [ serverBaseUrl, + availableRepos, setProgress, setViewMode, setGraph, @@ -1495,6 +1544,7 @@ const AppStateProviderInner = ({ children }: { children: ReactNode }) => { setProgress, projectName, setProjectName, + currentRepo, // Multi-repo switching serverBaseUrl, setServerBaseUrl, diff --git a/gitnexus-web/src/services/backend-client.ts b/gitnexus-web/src/services/backend-client.ts index 09b7dfffe..b4a9ebb2c 100644 --- a/gitnexus-web/src/services/backend-client.ts +++ b/gitnexus-web/src/services/backend-client.ts @@ -28,6 +28,13 @@ export interface BackendRepo { }; } +/** + * Canonical repo identity: the registry path. The display `name` is ambiguous + * across duplicate repo names (#2419); `repoPath` is the normalized field and + * `path` the legacy list-endpoint field. + */ +export const repoIdentity = (repo: BackendRepo): string => repo.repoPath ?? repo.path ?? repo.name; + export interface EnrichedSearchResult { filePath: string; score: number; @@ -535,7 +542,8 @@ export const probeBackend = async (): Promise => { export const fetchRepos = async (): Promise => { const response = await fetchWithTimeout(`${_backendUrl}/api/repos`); await assertOk(response); - return response.json() as Promise; + const repos = (await response.json()) as BackendRepo[]; + return repos.map((r) => ({ ...r, repoPath: r.repoPath ?? r.path })); }; /** Fetch repo metadata. @@ -891,7 +899,7 @@ export const cancelAnalyze = async (jobId: string): Promise => { export const streamAnalyzeProgress = ( jobId: string, onProgress: (progress: JobProgress) => void, - onComplete: (data: { repoName?: string }) => void, + onComplete: (data: { repoName?: string; repoPath?: string }) => void, onError: (error: string) => void, ): AbortController => { return streamSSE( @@ -940,7 +948,7 @@ export const cancelEmbeddings = async (jobId: string): Promise => { export const streamEmbeddingProgress = ( jobId: string, onProgress: (progress: JobProgress) => void, - onComplete: (data: { repoName?: string }) => void, + onComplete: (data: { repoName?: string; repoPath?: string }) => void, onError: (error: string) => void, ): AbortController => { return streamSSE(`${_backendUrl}/api/embed/${encodeURIComponent(jobId)}/progress`, { diff --git a/gitnexus-web/test/unit/backend-client-repo-identity.test.ts b/gitnexus-web/test/unit/backend-client-repo-identity.test.ts new file mode 100644 index 000000000..c0682f545 --- /dev/null +++ b/gitnexus-web/test/unit/backend-client-repo-identity.test.ts @@ -0,0 +1,108 @@ +/** + * Canonical repo identity (#2419). + * + * `repoIdentity` is the single identity helper for the web app: the registry + * path is canonical because the display `name` is ambiguous across duplicate + * repo names. `fetchRepos` must normalize legacy list-endpoint entries + * (`path` only) onto the `repoPath` field, mirroring `fetchRepoInfo`. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { __resetBreakerRegistry__ } from 'gitnexus-shared/test-helpers'; +import { + fetchRepos, + repoIdentity, + setBackendUrl, + type BackendRepo, +} from '../../src/services/backend-client'; + +const BASE = 'http://repo-identity.test:4747'; + +describe('repoIdentity fallback chain', () => { + it('prefers repoPath when present', () => { + const repo: BackendRepo = { + name: 'reels', + path: '/ws/group-a/reels', + repoPath: '/ws/group-b/reels', + indexedAt: '2026-07-10T00:00:00.000Z', + }; + expect(repoIdentity(repo)).toBe('/ws/group-b/reels'); + }); + + it('falls back to path when repoPath is absent', () => { + const repo: BackendRepo = { + name: 'reels', + path: '/ws/group-a/reels', + indexedAt: '2026-07-10T00:00:00.000Z', + }; + expect(repoIdentity(repo)).toBe('/ws/group-a/reels'); + }); + + it('falls back to the display name when neither path field is present', () => { + // Legacy payloads can omit both path fields at runtime even though the + // interface marks `path` required — assert the narrow legacy shape to + // exercise the final fallback without weakening the helper's signature. + const legacyRepo = { + name: 'reels', + indexedAt: '2026-07-10T00:00:00.000Z', + } as BackendRepo; + expect(repoIdentity(legacyRepo)).toBe('reels'); + }); +}); + +describe('fetchRepos repoPath normalization', () => { + beforeEach(() => { + __resetBreakerRegistry__(); + setBackendUrl(BASE); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('maps legacy path-only entries onto repoPath', async () => { + const legacyBody = JSON.stringify([ + { name: 'reels', path: '/ws/group-a/reels', indexedAt: '2026-07-10T00:00:00.000Z' }, + { name: 'docs', path: '/ws/group-b/docs', indexedAt: '2026-07-09T00:00:00.000Z' }, + ]); + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + expect(url).toContain('/api/repos'); + return new Response(legacyBody, { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }); + vi.stubGlobal('fetch', fetchMock); + + const repos = await fetchRepos(); + expect(repos).toMatchObject([ + { name: 'reels', path: '/ws/group-a/reels', repoPath: '/ws/group-a/reels' }, + { name: 'docs', path: '/ws/group-b/docs', repoPath: '/ws/group-b/docs' }, + ]); + }); + + it('keeps a server-provided repoPath over the legacy path field', async () => { + const body = JSON.stringify([ + { + name: 'reels', + path: '/ws/group-a/reels', + repoPath: '/ws/group-b/reels', + indexedAt: '2026-07-10T00:00:00.000Z', + }, + ]); + vi.stubGlobal( + 'fetch', + vi.fn( + async () => + new Response(body, { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ), + ); + + const repos = await fetchRepos(); + expect(repos).toMatchObject([{ name: 'reels', repoPath: '/ws/group-b/reels' }]); + }); +}); diff --git a/gitnexus-web/test/unit/code-references-panel.test.tsx b/gitnexus-web/test/unit/code-references-panel.test.tsx new file mode 100644 index 000000000..e5c6b3e88 --- /dev/null +++ b/gitnexus-web/test/unit/code-references-panel.test.tsx @@ -0,0 +1,73 @@ +import { render } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ReactNode } from 'react'; +import type { GraphNode } from 'gitnexus-shared'; +import { CodeReferencesPanel } from '../../src/components/CodeReferencesPanel'; +import { readFile } from '../../src/services/backend-client'; + +const fileNode: GraphNode = { + id: 'File:src/foo.ts', + label: 'File', + properties: { name: 'foo.ts', filePath: 'src/foo.ts' }, +}; + +// Mutable mock state: the useAppState factory closes over this object so each +// test can reassign fields (e.g. currentRepo) before rendering. +const appState = { + graph: null, + selectedNode: fileNode, + codeReferences: [], + removeCodeReference: vi.fn(), + clearCodeReferences: vi.fn(), + setSelectedNode: vi.fn(), + codeReferenceFocus: null, + projectName: 'reels', + currentRepo: undefined as string | undefined, +}; + +vi.mock('../../src/hooks/useAppState', () => ({ + useAppState: () => appState, +})); + +vi.mock('../../src/services/backend-client', () => ({ + readFile: vi.fn(), +})); + +vi.mock('react-syntax-highlighter', () => ({ + Prism: ({ children }: { children?: ReactNode }) => 
{children}
, +})); + +vi.mock('react-syntax-highlighter/dist/esm/styles/prism', () => ({ + vscDarkPlus: {}, +})); + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string) => key, + }), +})); + +describe('CodeReferencesPanel repo identity (#2420)', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(readFile).mockResolvedValue({ content: 'const a = 1;', totalLines: 1 }); + }); + + it('reads the selected file from the active repo path, not the display name', () => { + appState.currentRepo = '/ws/b/reels'; + appState.projectName = 'reels'; + + render(); + + expect(readFile).toHaveBeenCalledWith('src/foo.ts', { repo: '/ws/b/reels' }); + }); + + it('falls back to the project display name when no repo path is active', () => { + appState.currentRepo = undefined; + appState.projectName = 'reels'; + + render(); + + expect(readFile).toHaveBeenCalledWith('src/foo.ts', { repo: 'reels' }); + }); +}); diff --git a/gitnexus-web/test/unit/header.test.tsx b/gitnexus-web/test/unit/header.test.tsx index 1a45bd705..94190348b 100644 --- a/gitnexus-web/test/unit/header.test.tsx +++ b/gitnexus-web/test/unit/header.test.tsx @@ -1,12 +1,19 @@ import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { Header } from '../../src/components/Header'; +import { + deleteRepo, + fetchRepos, + startAnalyze, + streamAnalyzeProgress, +} from '../../src/services/backend-client'; import type { BackendRepo } from '../../src/services/backend-client'; vi.mock('../../src/hooks/useAppState', () => ({ useAppState: () => ({ projectName: 'reels', + currentRepo: '/workspace/group-b/reels', graph: null, graphMode: 'full', openChatPanel: vi.fn(), @@ -46,6 +53,8 @@ vi.mock('react-i18next', () => ({ if (key === 'header:repositories') return 'Repositories'; if (key === 'header:active') return 'Active'; if (key === 'header:reanalyzeRepo') return `Re-analyze ${options?.repoName ?? ''}`; + if (key === 'header:reanalyzingRepo') + return `Re-analyzing ${options?.repoName ?? ''}: ${options?.message ?? ''}`; if (key === 'header:deleteRepo') return `Delete ${options?.repoName ?? ''}`; if (key === 'header:analyzeNew') return 'Analyze new'; if (key === 'header:searchRepositories') return 'Search repositories...'; @@ -71,6 +80,16 @@ function makeRepo(index: number): BackendRepo { } describe('Header', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + // Reset the URL mutated by the delete handler's hygiene pass. + window.history.replaceState(null, '', '/'); + }); + it('keeps a large repository menu scrollable inside the viewport', () => { render(
makeRepo(index))} />); @@ -152,4 +171,157 @@ describe('Header', () => { expect(repoList).not.toHaveTextContent('feed_sync'); expect(repoList).not.toHaveTextContent('reels'); }); + + it('uses repository path identity when duplicate display names are present', async () => { + const onSwitchRepo = vi.fn(); + render( +
, + ); + + fireEvent.click(screen.getByRole('button', { name: /reels/i })); + + expect(screen.getAllByText('Active')).toHaveLength(1); + await userEvent.click(screen.getAllByText('reels')[1]); + + expect(onSwitchRepo).toHaveBeenCalledWith('/workspace/group-a/reels'); + }); + + it('deletes and falls back using repository path identity', async () => { + const onSwitchRepo = vi.fn(); + const updatedRepos = [{ ...makeRepo(2), name: 'reels', path: '/workspace/group-a/reels' }]; + vi.mocked(fetchRepos).mockResolvedValue(updatedRepos); + + render( +
, + ); + + fireEvent.click(screen.getByRole('button', { name: /reels/i })); + await userEvent.click(screen.getAllByTitle('Delete reels')[1]); + + expect(deleteRepo).toHaveBeenCalledWith('/workspace/group-b/reels'); + expect(onSwitchRepo).toHaveBeenCalledWith('/workspace/group-a/reels'); + }); + + it('strips repo, project and skipGraph from the URL before reloading after the last repo is deleted', async () => { + window.history.replaceState( + null, + '', + '/?repo=%2Fworkspace%2Fgroup-b%2Freels&project=reels&skipGraph=1', + ); + // jsdom's location.reload is own+non-configurable — replace the whole + // `location` accessor with a stub that delegates URL reads to the real + // Location (kept live by history.replaceState) and mocks reload. + const realLocation = window.location; + const reloadMock = vi.fn(); + vi.stubGlobal('location', { + get href() { + return realLocation.href; + }, + get search() { + return realLocation.search; + }, + reload: reloadMock, + }); + vi.mocked(fetchRepos).mockResolvedValue([]); + + render( +
, + ); + + fireEvent.click(screen.getByRole('button', { name: /reels/i })); + await userEvent.click(screen.getByTitle('Delete reels')); + + expect(reloadMock).toHaveBeenCalledTimes(1); + expect(window.location.search).not.toContain('repo='); + expect(window.location.search).not.toContain('project='); + expect(window.location.search).not.toContain('skipGraph'); + }); + + it('strips repo and project from the URL before falling back after deleting the active repo', async () => { + window.history.replaceState(null, '', '/?repo=%2Fworkspace%2Fgroup-b%2Freels&project=reels'); + // Capture the URL at the moment of the fallback switch — the stale + // identity must already be gone so a failed switch leaves nothing that + // restores the deleted repo on refresh (#2419). + const searchAtSwitch: string[] = []; + const onSwitchRepo = vi.fn(() => { + searchAtSwitch.push(window.location.search); + }); + vi.mocked(fetchRepos).mockResolvedValue([ + { ...makeRepo(2), name: 'reels', path: '/workspace/group-a/reels' }, + ]); + + render( +
, + ); + + fireEvent.click(screen.getByRole('button', { name: /reels/i })); + await userEvent.click(screen.getAllByTitle('Delete reels')[1]); + + expect(deleteRepo).toHaveBeenCalledWith('/workspace/group-b/reels'); + expect(onSwitchRepo).toHaveBeenCalledWith('/workspace/group-a/reels'); + expect(searchAtSwitch).toEqual(['']); + }); + + it('shows the display name, not the path identity, in the re-analyze progress label', async () => { + vi.mocked(startAnalyze).mockResolvedValue({ jobId: 'job-1', status: 'running' }); + vi.mocked(streamAnalyzeProgress).mockReturnValue(new AbortController()); + + render( +
, + ); + + fireEvent.click(screen.getByRole('button', { name: /reels/i })); + // Re-analyze the second duplicate-name row — `reanalyzing` becomes the + // path identity '/ws/b/reels', but the label must render the name. + await userEvent.click(screen.getAllByTitle('Re-analyze reels')[1]); + + const label = screen.getByText(/^Re-analyzing /); + expect(label.textContent).toMatch(/^Re-analyzing reels:/); + expect(label.textContent).not.toContain('/ws/b/reels'); + }); + + it('falls back to the path basename when the re-analyzing identity is no longer listed', async () => { + vi.mocked(startAnalyze).mockResolvedValue({ jobId: 'job-2', status: 'running' }); + vi.mocked(streamAnalyzeProgress).mockReturnValue(new AbortController()); + + const { rerender } = render( +
, + ); + + fireEvent.click(screen.getByRole('button', { name: /reels/i })); + await userEvent.click(screen.getByTitle('Re-analyze reels')); + + // The repo list refreshes while the re-analysis is still in flight and the + // identity disappears from it — the label degrades to the path basename. + rerender(
); + + const label = screen.getByText(/^Re-analyzing /); + expect(label.textContent).toMatch(/^Re-analyzing reels:/); + expect(label.textContent).not.toContain('/ws/b/reels'); + }); }); diff --git a/gitnexus-web/test/unit/initialize-agent-identity.test.tsx b/gitnexus-web/test/unit/initialize-agent-identity.test.tsx new file mode 100644 index 000000000..5e8eddbd7 --- /dev/null +++ b/gitnexus-web/test/unit/initialize-agent-identity.test.tsx @@ -0,0 +1,79 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { AppStateProvider, useAppState } from '../../src/hooks/useAppState'; +import { getActiveProviderConfig } from '../../src/core/llm/settings-service'; +import type { CodebaseContext } from '../../src/core/llm/context-builder'; + +// initializeAgent's heavy dynamic imports are stubbed — these tests only lock +// the identity-write rule, not agent behavior. +vi.mock('../../src/core/llm/context-builder', () => ({ + buildCodebaseContext: vi.fn( + async (): Promise => ({ + stats: { + projectName: 'stub', + fileCount: 0, + functionCount: 0, + classCount: 0, + interfaceCount: 0, + methodCount: 0, + }, + hotspots: [], + folderTree: '', + }), + ), +})); + +vi.mock('../../src/core/llm/agent', () => ({ + createGraphRAGAgent: vi.fn(() => ({})), +})); + +vi.mock('../../src/core/llm/settings-service', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getActiveProviderConfig: vi.fn(actual.getActiveProviderConfig) }; +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const withProvider = () => { + vi.mocked(getActiveProviderConfig).mockReturnValue({ + provider: 'openai', + model: 'gpt-4o', + apiKey: 'test-key', + }); +}; + +describe('initializeAgent repo-identity writes (#2419)', () => { + it('does not clobber the path identity when called with only a display name', async () => { + withProvider(); + const { result } = renderHook(() => useAppState(), { wrapper: AppStateProvider }); + + act(() => { + result.current.setCurrentRepo('/ws/b/reels'); + }); + + await act(async () => { + await result.current.initializeAgent('reels'); + }); + + // The pre-PR idiom initializeAgent(projectName) must no longer overwrite + // the path identity with an ambiguous display name. + expect(result.current.currentRepo).toBe('/ws/b/reels'); + }); + + it('writes the identity when opts.repo is provided', async () => { + withProvider(); + const { result } = renderHook(() => useAppState(), { wrapper: AppStateProvider }); + + act(() => { + result.current.setCurrentRepo('/ws/a/reels'); + }); + + await act(async () => { + await result.current.initializeAgent('reels', { repo: '/ws/b/reels' }); + }); + + expect(result.current.currentRepo).toBe('/ws/b/reels'); + }); +}); diff --git a/gitnexus-web/test/unit/load-graph-anyway.test.tsx b/gitnexus-web/test/unit/load-graph-anyway.test.tsx index 10c15c9cc..b7c41dd2d 100644 --- a/gitnexus-web/test/unit/load-graph-anyway.test.tsx +++ b/gitnexus-web/test/unit/load-graph-anyway.test.tsx @@ -1,6 +1,38 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { renderHook, act } from '@testing-library/react'; import { AppStateProvider, useAppState } from '../../src/hooks/useAppState'; +import { getActiveProviderConfig } from '../../src/core/llm/settings-service'; +import { buildCodebaseContext, type CodebaseContext } from '../../src/core/llm/context-builder'; + +// Capture initializeAgent's observable seam: buildCodebaseContext receives the +// effective project name that ends up in the agent's system prompt. +vi.mock('../../src/core/llm/context-builder', () => ({ + buildCodebaseContext: vi.fn( + async (): Promise => ({ + stats: { + projectName: 'stub', + fileCount: 0, + functionCount: 0, + classCount: 0, + interfaceCount: 0, + methodCount: 0, + }, + hotspots: [], + folderTree: '', + }), + ), +})); + +vi.mock('../../src/core/llm/agent', () => ({ + createGraphRAGAgent: vi.fn(() => ({})), +})); + +vi.mock('../../src/core/llm/settings-service', async (importOriginal) => { + const actual = await importOriginal(); + // Wrap with the real implementation so tests without an explicit override + // keep today's no-provider (null) behavior. + return { ...actual, getActiveProviderConfig: vi.fn(actual.getActiveProviderConfig) }; +}); afterEach(() => { vi.restoreAllMocks(); @@ -170,6 +202,81 @@ describe('loadGraphAnyway (chat-only escape hatch, #2178)', () => { expect(result.current.graphMode).toBe('chatOnly'); }); + it('re-initializes the agent with the looked-up display name and the path identity', async () => { + vi.mocked(getActiveProviderConfig).mockReturnValue({ + provider: 'openai', + model: 'gpt-4o', + apiKey: 'test-key', + }); + const fetchMock = vi.fn((url: string) => { + if (url.includes('/api/repo')) return Promise.resolve(repoInfoResponse()); + if (url.includes('/api/graph')) return Promise.resolve(graphNdjsonResponse()); + return Promise.resolve( + new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }), + ); + }); + vi.stubGlobal('fetch', fetchMock); + + const { result } = renderHook(() => useAppState(), { wrapper: AppStateProvider }); + act(() => { + result.current.setServerBaseUrl('http://localhost:4747'); + result.current.setAvailableRepos([ + { + name: 'reels-display', + path: '/r/big-repo', + repoPath: '/r/big-repo', + indexedAt: '2026-06-13T00:00:00Z', + }, + ]); + result.current.setCurrentRepo('/r/big-repo'); + result.current.setGraphMode('chatOnly'); + }); + + await act(async () => { + await result.current.loadGraphAnyway(); + }); + + // The agent prompt gets the human-readable display name — never the + // absolute path, and never the 'project' literal that initializeAgent's + // empty-deps closure would fall back to (projectName is trapped at ''). + expect(vi.mocked(buildCodebaseContext)).toHaveBeenCalledWith( + expect.any(Function), + 'reels-display', + ); + // The repo identity itself stays the path (threaded via opts.repo). + expect(result.current.currentRepo).toBe('/r/big-repo'); + }); + + it('falls back to the identity basename for the agent prompt when the repo list misses it', async () => { + vi.mocked(getActiveProviderConfig).mockReturnValue({ + provider: 'openai', + model: 'gpt-4o', + apiKey: 'test-key', + }); + const fetchMock = vi.fn((url: string) => { + if (url.includes('/api/repo')) return Promise.resolve(repoInfoResponse()); + if (url.includes('/api/graph')) return Promise.resolve(graphNdjsonResponse()); + return Promise.resolve( + new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }), + ); + }); + vi.stubGlobal('fetch', fetchMock); + + const { result } = renderHook(() => useAppState(), { wrapper: AppStateProvider }); + act(() => { + result.current.setServerBaseUrl('http://localhost:4747'); + result.current.setCurrentRepo('/r/big-repo'); + result.current.setGraphMode('chatOnly'); + }); + + await act(async () => { + await result.current.loadGraphAnyway(); + }); + + expect(vi.mocked(buildCodebaseContext)).toHaveBeenCalledWith(expect.any(Function), 'big-repo'); + expect(result.current.currentRepo).toBe('/r/big-repo'); + }); + it('does not throw or apply state when unmounted mid-load', async () => { let resolveGraph: (r: Response) => void = () => {}; const graphPromise = new Promise((res) => { diff --git a/gitnexus-web/test/unit/repo-analyzer-complete-identity.test.tsx b/gitnexus-web/test/unit/repo-analyzer-complete-identity.test.tsx new file mode 100644 index 000000000..ce0089c87 --- /dev/null +++ b/gitnexus-web/test/unit/repo-analyzer-complete-identity.test.tsx @@ -0,0 +1,103 @@ +/** + * Analyze completion: display vs identity split (PR #2420 review R2/R7). + * + * The SSE complete event may carry `repoPath` (the analyzed path). RepoAnalyzer + * must pass that IDENTITY to onComplete — so the post-analyze reconnect targets + * the exact repo even when basenames collide — while the done screen keeps + * rendering the display NAME and never shows an absolute path. Old servers + * omit repoPath; the name fallback must be preserved. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, fireEvent, render, screen } from '@testing-library/react'; +import { RepoAnalyzer } from '../../src/components/RepoAnalyzer'; +import { i18nReady } from '../../src/i18n'; +import { + cancelAnalyze, + streamAnalyzeProgress, + uploadFolder, +} from '../../src/services/backend-client'; + +vi.mock('../../src/services/backend-client', () => ({ + startAnalyze: vi.fn(), + cancelAnalyze: vi.fn(), + streamAnalyzeProgress: vi.fn(), + uploadFolder: vi.fn(), +})); + +const JOB = { jobId: 'job-1', status: 'queued' }; + +type CompleteData = { repoName?: string; repoPath?: string }; + +beforeEach(async () => { + await i18nReady; + vi.clearAllMocks(); + vi.mocked(cancelAnalyze).mockResolvedValue(undefined as never); + vi.mocked(uploadFolder).mockResolvedValue(JOB); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +/** + * Render RepoAnalyzer, drive a folder-upload analyze to the SSE stream, and + * return the onComplete spy plus the captured SSE complete callback. Uses fake + * timers because completion holds a ~1200ms timer before firing onComplete. + */ +async function startTrackedJob() { + let sseComplete: ((data: CompleteData) => void) | undefined; + vi.mocked(streamAnalyzeProgress).mockImplementation((_jobId, _onProgress, onComplete) => { + sseComplete = onComplete; + return new AbortController(); + }); + + vi.useFakeTimers(); + const onDone = vi.fn<(repoIdentity: string) => void>(); + render(); + fireEvent.click(screen.getByRole('tab', { name: 'Local Folder' })); + fireEvent.change(screen.getByTestId('folder-upload-input'), { + target: { files: [new File(['x'], 'a.ts')] }, + }); + // Flush the upload promise so trackJob subscribes to the SSE stream. + await act(async () => {}); + expect(streamAnalyzeProgress).toHaveBeenCalledTimes(1); + + return { onDone, complete: (data: CompleteData) => sseComplete?.(data) }; +} + +describe('analyze completion identity', () => { + it('passes repoPath to onComplete but renders only the display name', async () => { + const { onDone, complete } = await startTrackedJob(); + + act(() => { + complete({ repoName: 'reels', repoPath: '/ws/b/reels' }); + }); + + // Done screen shows the display name, never the absolute path. + expect(screen.getByText('reels')).toBeInTheDocument(); + expect(screen.queryByText('/ws/b/reels')).toBeNull(); + + // onComplete fires after the ~1200ms done-screen dwell, with the identity. + expect(onDone).not.toHaveBeenCalled(); + act(() => { + vi.advanceTimersByTime(1200); + }); + expect(onDone).toHaveBeenCalledTimes(1); + expect(onDone).toHaveBeenCalledWith('/ws/b/reels'); + }); + + it('falls back to the display name when the server omits repoPath', async () => { + const { onDone, complete } = await startTrackedJob(); + + act(() => { + complete({ repoName: 'reels' }); + }); + + expect(screen.getByText('reels')).toBeInTheDocument(); + act(() => { + vi.advanceTimersByTime(1200); + }); + expect(onDone).toHaveBeenCalledTimes(1); + expect(onDone).toHaveBeenCalledWith('reels'); + }); +}); diff --git a/gitnexus-web/test/unit/switch-repo-url.test.tsx b/gitnexus-web/test/unit/switch-repo-url.test.tsx new file mode 100644 index 000000000..d16af7a70 --- /dev/null +++ b/gitnexus-web/test/unit/switch-repo-url.test.tsx @@ -0,0 +1,68 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { AppStateProvider, useAppState } from '../../src/hooks/useAppState'; + +afterEach(() => { + vi.restoreAllMocks(); + // Reset the URL mutated by switchRepo's persistence. + window.history.replaceState(null, '', '/'); +}); + +// Duplicate-display-name repo: `name` alone cannot identify it (#2419), so the +// URL must carry the server-resolved path identity alongside the display name. +const repoInfoResponse = () => + new Response( + JSON.stringify({ + name: 'reels', + path: '/ws/group-b/reels', + repoPath: '/ws/group-b/reels', + indexedAt: '2026-07-10T00:00:00Z', + stats: { nodes: 300_000, edges: 600_000 }, + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + +describe('switchRepo URL persistence (#2419)', () => { + it('writes both the server-resolved repo path and the display name on success', async () => { + const fetchMock = vi.fn((url: string) => { + if (url.includes('/api/repo')) return Promise.resolve(repoInfoResponse()); + return Promise.resolve( + new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }), + ); + }); + vi.stubGlobal('fetch', fetchMock); + + const { result } = renderHook(() => useAppState(), { wrapper: AppStateProvider }); + act(() => { + result.current.setServerBaseUrl('http://localhost:4747'); + }); + + await act(async () => { + await result.current.switchRepo('/ws/group-b/reels'); + }); + + expect(window.location.search).toContain('repo=%2Fws%2Fgroup-b%2Freels'); + expect(window.location.search).toContain('project=reels'); + // A deliberate switch drops any per-repo skipGraph override (#2178). + expect(window.location.search).not.toContain('skipGraph'); + }); + + it('leaves the URL unchanged when the connect fails', async () => { + window.history.replaceState(null, '', '/?repo=%2Fws%2Fgroup-a%2Freels&project=reels'); + const fetchMock = vi.fn(() => Promise.reject(new Error('connection refused'))); + vi.stubGlobal('fetch', fetchMock); + + const { result } = renderHook(() => useAppState(), { wrapper: AppStateProvider }); + act(() => { + result.current.setServerBaseUrl('http://localhost:4747'); + }); + + await act(async () => { + await result.current.switchRepo('/ws/group-b/reels'); + }); + + // The failed target must not poison the URL — a refresh still restores + // the previously connected repo. + expect(window.location.search).toBe('?repo=%2Fws%2Fgroup-a%2Freels&project=reels'); + }); +}); diff --git a/gitnexus-web/test/unit/url-restore.test.ts b/gitnexus-web/test/unit/url-restore.test.ts new file mode 100644 index 000000000..e5b5bc349 --- /dev/null +++ b/gitnexus-web/test/unit/url-restore.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it, vi } from 'vitest'; + +// The helper lives in App.tsx, whose import chain pulls in WebGL-backed +// rendering (sigma via GraphCanvas) that jsdom cannot load — stub it out; +// this suite only exercises the pure pickRestoreRepo helper. +vi.mock('../../src/components/GraphCanvas', () => ({ + GraphCanvas: () => null, +})); + +import { pickRestoreRepo } from '../../src/App'; + +describe('pickRestoreRepo (URL restore param preference, #2419)', () => { + it('prefers the repo path identity when both params are present', () => { + const params = new URLSearchParams('repo=%2Fws%2Fgroup-b%2Freels&project=reels'); + + expect(pickRestoreRepo(params)).toBe('/ws/group-b/reels'); + }); + + it('falls back to the project display name for legacy project-only URLs', () => { + const params = new URLSearchParams('project=reels'); + + expect(pickRestoreRepo(params)).toBe('reels'); + }); + + it('uses the repo path identity when only repo is present', () => { + const params = new URLSearchParams('repo=%2Fws%2Fgroup-b%2Freels'); + + expect(pickRestoreRepo(params)).toBe('/ws/group-b/reels'); + }); + + it('returns undefined when neither param is present', () => { + const params = new URLSearchParams('server=http%3A%2F%2Flocalhost%3A4747'); + + expect(pickRestoreRepo(params)).toBeUndefined(); + }); +}); diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index 03f4673da..5d6b177f5 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -74,6 +74,11 @@ const PLATFORM_LOGIC = [ // (macos), so the header parsing is proven on genuine binaries, not synthetic // buffers (the ubuntu suite covers the ELF path). 'test/integration/extension-binary-real.test.ts', + // Server repo resolver branches on path shape (path.isAbsolute, backslash + // detection) and canonicalizePath/realpathSync, all of which differ between + // POSIX and Windows — the fail-closed path-claim semantics must hold on the + // real windows-latest path implementation (#2419/#2420). + 'test/unit/server-api-repo-resolution.test.ts', ]; // Native LadybugDB integration tests — exercise the @ladybugdb/core diff --git a/gitnexus/src/server/analyze-launch.ts b/gitnexus/src/server/analyze-launch.ts index 87a463ca0..b505cdfcf 100644 --- a/gitnexus/src/server/analyze-launch.ts +++ b/gitnexus/src/server/analyze-launch.ts @@ -11,10 +11,17 @@ */ import path from 'path'; +import { existsSync, statSync } from 'node:fs'; import { fork } from 'child_process'; import { fileURLToPath, pathToFileURL } from 'url'; import { createRequire } from 'node:module'; -import { getStoragePath } from '../storage/repo-manager.js'; +import { + canonicalizePath, + getStoragePath, + INDEX_METADATA_FILE, + listRegisteredRepos, + registryPathEquals, +} from '../storage/repo-manager.js'; import { logger } from '../core/logger.js'; import type { JobManager } from './analyze-job.js'; import type { WorkerMessage } from './analyze-worker.js'; @@ -26,6 +33,12 @@ export interface LaunchDeps { backend: { init: () => Promise }; acquireRepoLock: (key: string) => string | null; releaseRepoLock: (key: string) => void; + /** + * Drops the server's cached LadybugDB handle (closeLbug). The worker + * process rewrites the repo's DB files on disk, so a connection opened + * before the rewrite keeps reading the pre-rewrite state until evicted. + */ + closeDbHandle: () => Promise; } export interface LaunchOptions { @@ -37,14 +50,88 @@ export interface LaunchOptions { const MAX_WORKER_RETRIES = 2; +/** + * The worker reports `complete` over IPC before its on-disk finalization + * (LadybugDB checkpoint + native handle release + metadata write) is visible + * at `getStoragePath(targetPath)` — observed up to ~6.5s behind the IPC + * message. Opening the database inside that window is what the pre-IPC + * ordering was meant to prevent and is actively dangerous: reads fail with + * binder errors or return an empty graph, the open can quarantine the + * in-flight WAL, and the native layer racing the rewrite has crashed the + * whole server (SIGSEGV-class exit, no output) on slow CI runners. + */ +const FINALIZE_SETTLE_TIMEOUT_MS = 60_000; +const FINALIZE_SETTLE_POLL_MS = 200; + +/** + * Resolve once the analyzed repo's index is settled at `storagePath`: the + * LadybugDB file and metadata both exist AND were (re)written by THIS job + * (mtime >= jobStartMs — bare existence is not enough, a re-analysis leaves + * the previous index in place while it works), and no transient WAL/shadow/ + * checkpoint sidecars remain (the worker's native close has finished). + * + * Never rejects. Timing out logs and proceeds (pre-gate behavior) rather + * than failing a job whose analysis genuinely succeeded — e.g. a no-op + * non-force analyze legitimately rewrites nothing. + */ +/** + * Look up the analyzed repo's registered storage path. The request's + * user-provided path is used only as a comparison key; the filesystem probes + * below run against the registry's own `storagePath` — the server-owned + * record readers resolve through, and not a user-controlled value + * (CodeQL js/path-injection). + */ +const registeredStoragePath = async (targetPath: string): Promise => { + const target = canonicalizePath(path.resolve(targetPath)); + const entries = await listRegisteredRepos(); + const entry = entries.find((e) => registryPathEquals(canonicalizePath(e.path), target)); + return entry?.storagePath ?? null; +}; + +const waitForSettledIndex = async (targetPath: string, jobStartMs: number): Promise => { + const settled = (storagePath: string): boolean => { + try { + const lbugStat = statSync(path.join(storagePath, 'lbug')); + const metaStat = statSync(path.join(storagePath, INDEX_METADATA_FILE)); + return ( + lbugStat.mtimeMs >= jobStartMs && + metaStat.mtimeMs >= jobStartMs && + ['lbug.wal', 'lbug.shadow', 'lbug.wal.checkpoint'].every( + (f) => !existsSync(path.join(storagePath, f)), + ) + ); + } catch { + return false; // not written yet + } + }; + const deadline = Date.now() + FINALIZE_SETTLE_TIMEOUT_MS; + for (;;) { + // Re-resolved each round: the worker registers the repo as part of the + // finalization this gate is waiting out. + const storagePath = await registeredStoragePath(targetPath); + if (storagePath && settled(storagePath)) return; + if (Date.now() > deadline) { + logger.warn( + { targetPath }, + 'analyze finalization not visible after timeout; completing job anyway', + ); + return; + } + await new Promise((resolve) => setTimeout(resolve, FINALIZE_SETTLE_POLL_MS)); + } +}; + export function createLaunchAnalysisWorker(deps: LaunchDeps) { - const { jobManager, backend, acquireRepoLock, releaseRepoLock } = deps; + const { jobManager, backend, acquireRepoLock, releaseRepoLock, closeDbHandle } = deps; return function launchAnalysisWorker( job: { id: string }, targetPath: string, opts: LaunchOptions, ): void { + // For waitForSettledIndex: files (re)written by this job have mtimes at or + // after this instant. Taken before the fork so no worker write predates it. + const jobStartMs = Date.now(); // Acquire shared repo lock (keyed on storagePath to match embed handler) const analyzeLockKey = getStoragePath(targetPath); const lockErr = acquireRepoLock(analyzeLockKey); @@ -95,10 +182,17 @@ export function createLaunchAnalysisWorker(deps: LaunchDeps) { }); } else if (msg.type === 'complete') { releaseRepoLock(analyzeLockKey); - // Reinitialize backend BEFORE marking complete — ensures the new repo - // is queryable when the client receives the SSE complete event. - backend - .init() + // Before marking complete: (1) wait for the worker's on-disk + // finalization to settle (see waitForSettledIndex), (2) evict the + // cached DB handle — same invalidation DELETE /api/repo performs, a + // handle opened before the rewrite reads pre-rewrite state — and + // only then (3) reinitialize the backend. This makes the ordering + // comment below true in practice: the repo is actually queryable + // when the client receives the SSE complete event. + waitForSettledIndex(targetPath, jobStartMs) + .then(() => closeDbHandle()) + .catch(() => {}) // best-effort: eviction failure must not fail the job + .then(() => backend.init()) .then(() => { jobManager.updateJob(job.id, { status: 'complete', repoName: msg.result.repoName }); }) @@ -111,6 +205,8 @@ export function createLaunchAnalysisWorker(deps: LaunchDeps) { }); } else if (msg.type === 'error') { releaseRepoLock(analyzeLockKey); + // A failed (force) analyze may still have rewritten DB files first. + void closeDbHandle().catch(() => {}); jobManager.updateJob(job.id, { status: 'failed', error: msg.message }); } }); diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 68fa6f872..218c6e211 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -13,7 +13,15 @@ import cors from 'cors'; import path from 'path'; import fs from 'fs/promises'; import { createRequire } from 'node:module'; -import { loadMeta, listRegisteredRepos, getStoragePath } from '../storage/repo-manager.js'; +import { + canonicalizePath, + cloneDirBelongsToEntry, + loadMeta, + listRegisteredRepos, + getStoragePath, + registryPathEquals, + type RegistryEntry, +} from '../storage/repo-manager.js'; import { executeQuery, executePrepared, @@ -458,8 +466,13 @@ export const streamGraphNdjson = async ( /** * Mount an SSE progress endpoint for a JobManager. * Handles: initial state, terminal events, heartbeat, event IDs, client disconnect. + * + * Terminal payloads carry `repoPath` (the analyzed path) alongside the display + * `repoName` so clients can reconnect by path identity — with duplicate + * basenames, a name-only reconnect resolves to the first same-named sibling. + * Exported for unit tests that lock the wire payload shape. */ -const mountSSEProgress = (app: express.Express, routePath: string, jm: JobManager) => { +export const mountSSEProgress = (app: express.Express, routePath: string, jm: JobManager) => { app.get(routePath, (req, res) => { let jobId: string; try { @@ -492,6 +505,7 @@ const mountSSEProgress = (app: express.Express, routePath: string, jm: JobManage res.write( `id: ${eventId}\nevent: ${job.status}\ndata: ${JSON.stringify({ repoName: job.repoName, + repoPath: job.repoPath, error: job.error, })}\n\n`, ); @@ -518,6 +532,7 @@ const mountSSEProgress = (app: express.Express, routePath: string, jm: JobManage res.write( `id: ${eventId}\nevent: ${progress.phase}\ndata: ${JSON.stringify({ repoName: eventJob?.repoName, + repoPath: eventJob?.repoPath, error: eventJob?.error, })}\n\n`, ); @@ -561,6 +576,56 @@ const requestedRepo = (req: express.Request): string | undefined => { return undefined; }; +const repoParamBasename = (repoName: string): string => + repoName.replace(/\\/g, '/').split('/').filter(Boolean).pop() ?? repoName; + +/** + * Resolve a `?repo=` request param against the registry in two tiers: + * + * 1. Path claim — any input containing a separator ('/' or '\\', which + * cover path.sep on every platform) is treated as a path claim and + * resolved by canonical registry path ONLY. A miss fails closed + * (null, never a basename fallback) so a stale or wrong path can + * never silently retarget a same-named sibling repo (#2419). + * Within this tier, only absolute or Windows-shaped ('\\') claims + * are worth canonicalizing; relative claims like 'org/name' or + * './repo' are rejected immediately WITHOUT touching the filesystem + * — canonicalizing them would run an attacker-influenced + * CWD-relative realpathSync probe on un-rate-limited GET routes, + * and no legitimate caller sends relative paths. + * 2. Name fallback — bare names (no separators) keep the legacy + * basename/name match for older callers. + */ +export const resolveRegisteredRepoEntry = ( + repos: RegistryEntry[], + repoName?: string, +): RegistryEntry | null => { + if (!repoName) return repos[0] ?? null; + + const looksLikePath = + path.isAbsolute(repoName) || repoName.includes('/') || repoName.includes('\\'); + + if (looksLikePath) { + // Relative path claims fail closed with zero filesystem probes. + if (!path.isAbsolute(repoName) && !repoName.includes('\\')) return null; + + const requestedPath = canonicalizePath(repoName); + const pathMatch = repos.find((r) => + registryPathEquals(canonicalizePath(r.path), requestedPath), + ); + if (pathMatch) return pathMatch; + return null; + } + + const normalizedName = repoParamBasename(repoName); + + return ( + repos.find((r) => r.name === normalizedName) || + repos.find((r) => r.name.toLowerCase() === normalizedName.toLowerCase()) || + null + ); +}; + /** * Handle a GET /api/file request body. Extracted from createServer's route * registration so it can be unit-tested without spinning up an HTTP server @@ -821,6 +886,7 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => backend, acquireRepoLock, releaseRepoLock, + closeDbHandle: closeLbug, }); /** @@ -833,20 +899,9 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => // Pass `req` to enable early exit if the client disconnects during the hold-queue wait. const resolveRepo = async (repoName?: string, isRetry = false, req?: any): Promise => { const repos = await listRegisteredRepos(); - let found = null; + const found = resolveRegisteredRepoEntry(repos, repoName); - // Normalize: if a full path is passed, extract just the basename. - // e.g. "C:\Users\LENOVO\.gitnexus\repos\todo.txt-cli" -> "todo.txt-cli" - const normalizedName = repoName ? path.basename(repoName) : undefined; - - if (normalizedName) { - found = - repos.find((r) => r.name === normalizedName) || - repos.find((r) => r.name.toLowerCase() === normalizedName.toLowerCase()) || - null; - } else if (repos.length > 0) { - found = repos[0]; // default to first repo - } + const normalizedName = repoName ? repoParamBasename(repoName) : undefined; // If not yet in the registry, check whether a background job is actively cloning or // analyzing this repo. Hold the connection open (up to 5 minutes) until it completes. @@ -885,7 +940,7 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => if (currentJob.status === 'complete') { await backend.init(); const freshRepos = await listRegisteredRepos(); - return freshRepos.find((r) => r.name === normalizedName) || null; + return resolveRegisteredRepoEntry(freshRepos, repoName); } await new Promise((r) => setTimeout(r, 1000)); } @@ -906,7 +961,7 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => ); } await backend.init(); - return await resolveRepo(normalizedName, true, req); + return await resolveRepo(repoName, true, req); } return found; @@ -966,6 +1021,7 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => repos.map((r) => ({ name: r.name, path: r.path, + repoPath: r.path, indexedAt: r.indexedAt, lastCommit: r.lastCommit, stats: r.stats, @@ -977,7 +1033,11 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => }); // Get repo info - app.get('/api/repo', async (req, res) => { + // Rate-limited (CodeQL js/missing-rate-limiting): resolveRepo canonicalizes + // the attacker-supplied ?repo= param (realpathSync probe for absolute / + // Windows-shaped claims). Default 60 rpm/IP — web callers hit this route + // only on connect/switch, never in a polling loop. + app.get('/api/repo', createRouteLimiter(), async (req, res) => { try { const entry = await resolveRepo(requestedRepo(req), false, req); if (!entry) { @@ -1049,7 +1109,10 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => } catch { /* repo name not eligible for a clone dir (local repo) */ } - if (cloneDir) { + // Only remove the clone dir when it is *this* entry's path — a local + // repo registered under the same name would otherwise take a cloned + // sibling's checkout down with it (see cloneDirBelongsToEntry). + if (cloneDir && cloneDirBelongsToEntry(cloneDir, entry.path)) { try { const stat = await fs.stat(cloneDir); if (stat.isDirectory()) { diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 47e336a7f..4c4e59f18 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -82,6 +82,20 @@ export const canonicalizePath = (p: string): string => { export const registryPathEquals = (a: string, b: string): boolean => process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; +/** + * Does the clone dir derived from an entry's *name* actually belong to that + * entry? Registry names are not unique across storage locations: a cloned + * repo under `~/.gitnexus/repos/` and a local repo registered under the + * same name share a `getCloneDir(entry.name)` result. The server's delete + * handler must therefore never remove the clone dir based on the name alone — + * only when the entry's own `path` resolves to that dir (mirroring its step-2b + * rule that cleanup is driven off `entry.path`, so a same-named sibling's + * clone is never removed). Both sides are canonicalised so symlinked or + * differently-spelled forms of the same dir still match. + */ +export const cloneDirBelongsToEntry = (cloneDir: string, entryPath: string): boolean => + registryPathEquals(canonicalizePath(cloneDir), canonicalizePath(entryPath)); + export interface RepoMeta { repoPath: string; lastCommit: string; diff --git a/gitnexus/test/unit/rate-limit.test.ts b/gitnexus/test/unit/rate-limit.test.ts index 2f52e9df8..ac977ab9a 100644 --- a/gitnexus/test/unit/rate-limit.test.ts +++ b/gitnexus/test/unit/rate-limit.test.ts @@ -240,6 +240,10 @@ describe('production routes — rate-limit middleware wiring', () => { expect(apiSource).toMatch(/app\.delete\('\/api\/repo',\s*createRouteLimiter\(/); }); + it('GET /api/repo is wired with createRouteLimiter', () => { + expect(apiSource).toMatch(/app\.get\('\/api\/repo',\s*createRouteLimiter\(/); + }); + it('POST /api/analyze is wired with createRouteLimiter', () => { // Tolerate Prettier wrapping the registration across lines (it does once // the route carries extra middleware like requireLocalhostOrigin). diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 02954b491..a269daab2 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -28,6 +28,7 @@ import { listRegisteredRepos, resolveRegistryEntry, canonicalizePath, + cloneDirBelongsToEntry, assertSafeStoragePath, RegistryNameCollisionError, RegistryNotFoundError, @@ -1408,6 +1409,33 @@ describe('canonicalizePath (#1003)', () => { }); }); +describe('cloneDirBelongsToEntry', () => { + it('returns true when the clone dir and entry.path canonicalize to the same dir', () => { + // Non-canonical spelling of a REAL path (same trick as the + // resolveRegistryEntry backward-compat test): raw concat keeps the + // strings unequal until canonicalizePath runs. + const realDir = process.cwd(); + const nonCanonical = realDir + path.sep + '.'; + expect(nonCanonical).not.toBe(realDir); + expect(cloneDirBelongsToEntry(nonCanonical, realDir)).toBe(true); + }); + + it('returns false when the entry.path lives elsewhere than the clone dir', () => { + // The delete-handler scenario: entry B is a local repo whose name + // collides with clone A's — its path must not claim A's clone dir. + const cloneDir = path.join(os.tmpdir(), 'gitnexus-clones', 'reels'); + const entryPath = path.join(os.tmpdir(), 'local', 'reels'); + expect(cloneDirBelongsToEntry(cloneDir, entryPath)).toBe(false); + }); + + it('compares nonexistent paths without throwing (realpath falls back to path.resolve)', () => { + // Neither side exists on disk — canonicalizePath must fall back to + // path.resolve on both, so equal strings still compare equal. + const ghost = path.join(os.tmpdir(), 'gnx-never-exists-____', 'clone-dir'); + expect(cloneDirBelongsToEntry(ghost, ghost)).toBe(true); + }); +}); + describe('resolveRegistryEntry backward-compat with non-canonical stored paths (#1003)', () => { it('matches a stored entry even when the target was passed in canonical form', async () => { // Simulate the bug-producing scenario without depending on a real diff --git a/gitnexus/test/unit/server-api-repo-resolution.test.ts b/gitnexus/test/unit/server-api-repo-resolution.test.ts new file mode 100644 index 000000000..4aad2fbb4 --- /dev/null +++ b/gitnexus/test/unit/server-api-repo-resolution.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from 'vitest'; +import { resolveRegisteredRepoEntry } from '../../src/server/api.js'; +import type { RegistryEntry } from '../../src/storage/repo-manager.js'; + +const entry = (overrides: Partial): RegistryEntry => ({ + name: 'repo', + path: '/tmp/repo', + storagePath: '/tmp/repo/.gitnexus', + indexedAt: '2026-07-09T00:00:00.000Z', + lastCommit: 'deadbeef', + ...overrides, +}); + +describe('resolveRegisteredRepoEntry', () => { + it('resolves an explicit alias by exact registry path before basename fallback', () => { + const aliased = entry({ + name: 'e2e-mini-repo', + path: '/tmp/gitnexus-e2e-repo', + storagePath: '/tmp/gitnexus-e2e-repo/.gitnexus', + }); + + expect(resolveRegisteredRepoEntry([aliased], '/tmp/gitnexus-e2e-repo')).toBe(aliased); + }); + + it('falls back to basename/name matching for older callers', () => { + const repo = entry({ name: 'e2e-mini-repo' }); + + expect(resolveRegisteredRepoEntry([repo], 'e2e-mini-repo')).toBe(repo); + expect(resolveRegisteredRepoEntry([repo], 'E2E-MINI-REPO')).toBe(repo); + }); + + it('does not fall back to a duplicate basename after a path-shaped miss', () => { + const first = entry({ + name: 'service', + path: '/tmp/first/service', + storagePath: '/tmp/first/service/.gitnexus', + }); + const second = entry({ + name: 'service', + path: '/tmp/second/service', + storagePath: '/tmp/second/service/.gitnexus', + }); + + expect(resolveRegisteredRepoEntry([first, second], '/tmp/missing/service')).toBeNull(); + expect(resolveRegisteredRepoEntry([first, second], '/tmp/second/service')).toBe(second); + }); + + it('fails closed on relative slash input instead of basename fallback', () => { + const named = entry({ + name: 'name', + path: '/tmp/org/name', + storagePath: '/tmp/org/name/.gitnexus', + }); + + expect(resolveRegisteredRepoEntry([named], 'org/name')).toBeNull(); + }); + + it('fails closed on dot-relative input instead of name fallback', () => { + const repo = entry({ name: 'repo' }); + + expect(resolveRegisteredRepoEntry([repo], './repo')).toBeNull(); + }); + + it('returns the first-registered entry when a bare name matches two entries', () => { + // Documented legacy first-wins behavior: bare display names are ambiguous + // across duplicate-name registrations, and the resolver deliberately keeps + // returning the earliest registry entry (callers needing precision pass a path). + const first = entry({ + name: 'reels', + path: '/tmp/group-a/reels', + storagePath: '/tmp/group-a/reels/.gitnexus', + }); + const second = entry({ + name: 'reels', + path: '/tmp/group-b/reels', + storagePath: '/tmp/group-b/reels/.gitnexus', + }); + + expect(resolveRegisteredRepoEntry([first, second], 'reels')).toBe(first); + }); + + it('treats Windows-shaped input as a path claim and never falls back to basename', () => { + // The backslash makes 'C:\ws\reels' a path claim, so canonicalization must + // miss and the resolver must return null — NOT the same-named 'reels' entry. + // This expectation is platform-unconditional: on POSIX the drive-letter path + // canonicalizes to a nonexistent cwd-relative path, and on Windows CI + // C:\ws\reels genuinely does not exist, so both platforms must yield null. + const reels = entry({ + name: 'reels', + path: '/tmp/reels', + storagePath: '/tmp/reels/.gitnexus', + }); + + expect(resolveRegisteredRepoEntry([reels], 'C:\\ws\\reels')).toBeNull(); + }); + + it('defaults to the first registered repo when no name is requested', () => { + const first = entry({ + name: 'alpha', + path: '/tmp/alpha', + storagePath: '/tmp/alpha/.gitnexus', + }); + const second = entry({ + name: 'beta', + path: '/tmp/beta', + storagePath: '/tmp/beta/.gitnexus', + }); + + expect(resolveRegisteredRepoEntry([first, second], undefined)).toBe(first); + }); + + it('returns null for an empty registry when no name is requested', () => { + expect(resolveRegisteredRepoEntry([], undefined)).toBeNull(); + }); + + it('matches a bare name case-insensitively when no exact-case entry exists', () => { + // Regression guard for the fail-closed refactor: the case-insensitive + // bare-name fallback must survive the path-claim tightening. + const reels = entry({ + name: 'reels', + path: '/tmp/reels', + storagePath: '/tmp/reels/.gitnexus', + }); + + expect(resolveRegisteredRepoEntry([reels], 'REELS')).toBe(reels); + }); +}); diff --git a/gitnexus/test/unit/server-sse-payload.test.ts b/gitnexus/test/unit/server-sse-payload.test.ts new file mode 100644 index 000000000..898898f74 --- /dev/null +++ b/gitnexus/test/unit/server-sse-payload.test.ts @@ -0,0 +1,98 @@ +/** + * SSE terminal payload wire shape (mountSSEProgress). + * + * The `event: complete` payload must carry `repoPath` (the analyzed path) + * alongside the display `repoName` at BOTH terminal emit sites: + * (a) the already-terminal replay (job finished before the client subscribed) + * (b) the live subscription (job finishes while the client is connected) + * + * Clients reconnect by this identity after "Analyze new" — with duplicate + * basenames, a name-only payload makes the web UI connect to the first + * same-named sibling instead of the repo just analyzed (PR #2420 review R2). + */ +import express from 'express'; +import http from 'node:http'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { mountSSEProgress } from '../../src/server/api.js'; +import { JobManager } from '../../src/server/analyze-job.js'; + +const REPO_PATH = '/ws/b/reels'; +const REPO_NAME = 'reels'; + +/** Extract the parsed JSON payload of the `event: complete` SSE frame. */ +const parseCompletePayload = (body: string): unknown => { + const frame = body.split('\n\n').find((f) => f.includes('event: complete')); + expect(frame).toBeDefined(); + const dataLine = frame?.split('\n').find((line) => line.startsWith('data: ')); + expect(dataLine).toBeDefined(); + return JSON.parse(dataLine?.slice('data: '.length) ?? '{}') as unknown; +}; + +describe('mountSSEProgress terminal payload', () => { + let manager: JobManager; + let server: http.Server | undefined; + let baseUrl = ''; + + beforeEach(() => { + manager = new JobManager(); + const app = express(); + // Mirrors the production mount in createServer(). + mountSSEProgress(app, '/api/analyze/:jobId/progress', manager); + return new Promise((resolve) => { + server = app.listen(0, '127.0.0.1', () => { + const addr = server?.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + baseUrl = `http://127.0.0.1:${port}`; + resolve(); + }); + }); + }); + + afterEach(() => { + manager.dispose(); + return new Promise((resolve, reject) => { + if (!server) { + resolve(); + return; + } + server.close((err) => (err ? reject(err) : resolve())); + server = undefined; + }); + }); + + it('already-terminal replay includes repoName AND repoPath', async () => { + const job = manager.createJob({ repoPath: REPO_PATH }); + manager.updateJob(job.id, { status: 'complete', repoName: REPO_NAME }); + + const response = await fetch(`${baseUrl}/api/analyze/${job.id}/progress`); + const body = await response.text(); + + expect(body).toContain('event: complete'); + // Exact match locks the wire shape (error is undefined → omitted by JSON). + expect(parseCompletePayload(body)).toEqual({ + repoName: REPO_NAME, + repoPath: REPO_PATH, + }); + }); + + it('live subscription terminal event includes repoName AND repoPath', async () => { + const job = manager.createJob({ repoPath: REPO_PATH }); + + // fetch resolves once headers arrive — the handler has already subscribed + // to progress events by then (subscription happens synchronously). + const response = await fetch(`${baseUrl}/api/analyze/${job.id}/progress`); + manager.updateJob(job.id, { + status: 'analyzing', + progress: { phase: 'parsing', percent: 30, message: 'Parsing' }, + }); + manager.updateJob(job.id, { status: 'complete', repoName: REPO_NAME }); + + const body = await response.text(); + + expect(body).toContain('event: complete'); + expect(parseCompletePayload(body)).toEqual({ + repoName: REPO_NAME, + repoPath: REPO_PATH, + }); + }); +});