diff --git a/.changelog/NEXT.md b/.changelog/NEXT.md index 88bfb297..0ed34cf2 100644 --- a/.changelog/NEXT.md +++ b/.changelog/NEXT.md @@ -50,6 +50,7 @@ ## Fixed +- AI discovery now rejects unsafe batch settings, limits background runs to one per family database, and lets an active run be cancelled without leaving provider work behind. - **[issue-158] Unknown API routes now return JSON errors** — Requests to unrecognized `/api` paths receive a stable 404 error envelope instead of the browser app's HTML, while client-side navigation continues to use the SPA fallback. - Search results now keep their alphabetical ordering. The batch person-loader (`getPersonsBatch`) re-orders rows back to the requested order, fixing a regression where SQLite's `WHERE person_id IN (...)` returned rows in table order and silently discarded the search query's `ORDER BY display_name` (so the default, unsorted search view appeared randomly ordered). - Platform comparison now treats equivalent place spellings as matches: "Dallas, Texas, USA" vs "Dallas, Texas, United States" (and U.S.A. / United States of America / state abbreviations like TX vs Texas, UK vs United Kingdom, etc.) — no longer flagged as `different`. Place containment is now suffix-based, so "Texas" no longer falsely matches "Texarkana" diff --git a/client/src/services/api.ts b/client/src/services/api.ts index bc7c78f8..b8274721 100644 --- a/client/src/services/api.ts +++ b/client/src/services/api.ts @@ -787,6 +787,11 @@ export const api = { body: JSON.stringify(options || {}) }), + cancelDiscovery: (dbId: string) => + fetchJson<{ runId: string; message: string }>(`/ai-discovery/${dbId}/cancel`, { + method: 'POST', + }), + getDiscoveryProgress: (runId: string) => fetchJson(`/ai-discovery/progress/${runId}`), @@ -1066,7 +1071,7 @@ export interface DiscoveryResult { } export interface DiscoveryProgress { - status: 'pending' | 'running' | 'completed' | 'failed'; + status: 'pending' | 'running' | 'completed' | 'failed' | 'cancelled'; totalPersons: number; analyzedPersons: number; candidatesFound: number; diff --git a/docs/api.md b/docs/api.md index d14d0779..e0d3590f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -71,6 +71,17 @@ Backend runs on port 6374 by default. Application JSON endpoints use the envelop | GET | `/api/favorites/db/:dbId/tags` | Get tags for database | | GET | `/api/favorites/db/:dbId/sparse-tree` | Get sparse tree data | +## AI Discovery + +| Method | Path | Description | +|--------|------|-------------| +| POST | `/api/ai-discovery/:dbId/quick` | Analyze a small ancestor sample synchronously | +| POST | `/api/ai-discovery/:dbId/start` | Start one bounded background discovery run for this database | +| POST | `/api/ai-discovery/:dbId/cancel` | Cancel the active background discovery run | +| GET | `/api/ai-discovery/progress/:runId` | Get background discovery progress | + +`POST /api/ai-discovery/:dbId/start` accepts optional `{ batchSize, maxPersons }` values. Both must be positive integers; `batchSize` is capped at 100 and `maxPersons` at 1000. Defaults are 50 and 500 respectively. A second active run for the same database returns `409` with the active run ID. + ## Augmentation | Method | Path | Description | diff --git a/server/src/routes/ai-discovery.routes.ts b/server/src/routes/ai-discovery.routes.ts index 1e63d7d4..ec7d0e87 100644 --- a/server/src/routes/ai-discovery.routes.ts +++ b/server/src/routes/ai-discovery.routes.ts @@ -1,5 +1,10 @@ import { Router, Request, Response } from 'express'; -import { aiDiscoveryService } from '../services/ai-discovery.service.js'; +import { + aiDiscoveryService, + DiscoveryInputError, + DiscoveryRunConflictError, + normalizeFullDiscoveryOptions, +} from '../services/ai-discovery.service.js'; import { favoritesService } from '../services/favorites.service.js'; import { logger } from '../lib/logger.js'; import { asyncHandler } from '../utils/asyncHandler.js'; @@ -52,21 +57,47 @@ router.post('/:dbId/quick', asyncHandler(async (req: Request, res: Response) => */ router.post('/:dbId/start', asyncHandler(async (req: Request, res: Response) => { const { dbId } = req.params; - const { batchSize, maxPersons } = req.body; - - const result = await aiDiscoveryService.startDiscovery(dbId, { - batchSize, - maxPersons, - }).catch(err => { - res.status(500).json({ success: false, error: err.message }); - return null; - }); + let options; + try { + options = normalizeFullDiscoveryOptions(req.body ?? {}); + } catch (err) { + const message = err instanceof DiscoveryInputError ? err.message : 'Invalid discovery options'; + logger.warn('ai-discovery', `Rejected full discovery request dbId=${dbId}: ${message}`); + res.status(400).json({ success: false, error: message }); + return; + } - if (result !== null) { + try { + const result = await aiDiscoveryService.startDiscovery(dbId, options); res.json({ success: true, data: result }); + } catch (err) { + if (err instanceof DiscoveryRunConflictError) { + logger.warn('ai-discovery', `Rejected concurrent full discovery dbId=${dbId}, activeRunId=${err.runId}`); + res.status(409).json({ success: false, error: err.message, data: { runId: err.runId } }); + return; + } + const message = err instanceof Error ? err.message : 'Unable to start discovery'; + logger.error('ai-discovery', `Failed to start full discovery dbId=${dbId}: ${message}`); + res.status(500).json({ success: false, error: message }); } })); +/** + * Cancel the active full AI discovery run for a database. + * POST /api/ai-discovery/:dbId/cancel + */ +router.post('/:dbId/cancel', (req: Request, res: Response) => { + const { dbId } = req.params; + const cancelled = aiDiscoveryService.cancelDiscovery(dbId); + + if (!cancelled) { + res.status(404).json({ success: false, error: 'No active discovery run for this database' }); + return; + } + + res.json({ success: true, data: { ...cancelled, message: 'Cancellation requested' } }); +}); + /** * Get progress of a discovery run * GET /api/ai-discovery/progress/:runId diff --git a/server/src/services/ai-discovery.service.ts b/server/src/services/ai-discovery.service.ts index 4dfd7631..e10be7dd 100644 --- a/server/src/services/ai-discovery.service.ts +++ b/server/src/services/ai-discovery.service.ts @@ -18,7 +18,33 @@ function safeJsonParse(str: string): unknown { /** * Execute AI prompt using the configured AI toolkit provider */ -async function executeAiPrompt(prompt: string, timeoutMs = 300000): Promise { +class DiscoveryCancelledError extends Error { + constructor() { + super('Discovery cancelled'); + this.name = 'DiscoveryCancelledError'; + } +} + +export class DiscoveryInputError extends Error { + constructor(message: string) { + super(message); + this.name = 'DiscoveryInputError'; + } +} + +export class DiscoveryRunConflictError extends Error { + constructor(public readonly runId: string) { + super(`A discovery run is already active for this database (${runId}).`); + this.name = 'DiscoveryRunConflictError'; + } +} + +function throwIfCancelled(signal?: AbortSignal): void { + if (signal?.aborted) throw new DiscoveryCancelledError(); +} + +async function executeAiPrompt(prompt: string, timeoutMs = 300000, signal?: AbortSignal): Promise { + throwIfCancelled(signal); const startTime = Date.now(); const toolkit = getAIToolkit(); const { providers, runner } = toolkit.services; @@ -41,33 +67,70 @@ async function executeAiPrompt(prompt: string, timeoutMs = 300000): Promise { let output = ''; + let settled = false; - const onData = (text: string) => { - output += text; + let timeoutHandle: NodeJS.Timeout | undefined; + + const settle = (callback: () => void) => { + if (settled) return; + settled = true; + if (timeoutHandle) clearTimeout(timeoutHandle); + signal?.removeEventListener('abort', onAbort); + callback(); + }; + + const onData = (data: string | { text?: string; isReasoning?: boolean }) => { + if (typeof data !== 'string' && data.isReasoning) return; + output += typeof data === 'string' ? data : data.text ?? ''; }; const onComplete = (metadata: { success: boolean; error?: string; errorDetails?: string; duration?: number }) => { const elapsed = Date.now() - startTime; if (metadata.success) { logger.done('ai-discovery', `${activeProvider.name} completed in ${elapsed}ms, response: ${output.length} chars`); - resolve(output); + settle(() => resolve(output)); } else { const errorMsg = metadata.errorDetails || metadata.error || 'Unknown error'; logger.error('ai-discovery', `${activeProvider.name} failed after ${elapsed}ms: ${metadata.error || 'Unknown error'}`); if (metadata.errorDetails) { logger.error('ai-discovery', `Error details: ${metadata.errorDetails}`); } - reject(new Error(`AI run failed: ${errorMsg}`)); + settle(() => reject(new Error(`AI run failed: ${errorMsg}`))); } }; - if (provider.type === 'cli') { - runner.executeCliRun(runId, provider, prompt, process.cwd(), onData, onComplete, timeout || timeoutMs); - } else { - runner.executeApiRun(runId, provider, provider.defaultModel, prompt, process.cwd(), null, onData, onComplete); + const stopProviderRun = () => { + void runner.stopRun(runId).catch((err: Error) => { + logger.warn('ai-discovery', `Failed to stop provider run ${runId}: ${err.message}`); + }); + }; + + const onAbort = () => { + logger.warn('ai-discovery', `Cancelling provider run ${runId}`); + stopProviderRun(); + settle(() => reject(new DiscoveryCancelledError())); + }; + + signal?.addEventListener('abort', onAbort, { once: true }); + if (signal?.aborted) { + onAbort(); + return; } + + timeoutHandle = setTimeout(() => { + logger.error('ai-discovery', `Provider run ${runId} timed out after ${timeoutMs}ms`); + stopProviderRun(); + settle(() => reject(new Error(`AI discovery provider timed out after ${timeoutMs}ms`))); + }, timeoutMs); + + const execution = provider.type === 'cli' + ? runner.executeCliRun(runId, provider, prompt, process.cwd(), onData, onComplete, timeout || timeoutMs) + : runner.executeApiRun(runId, provider, provider.defaultModel, prompt, process.cwd(), null, onData, onComplete); + void execution.catch((err: Error) => settle(() => reject(err))); }); } @@ -93,7 +156,7 @@ export interface DiscoveryResult { } export interface DiscoveryProgress { - status: 'pending' | 'running' | 'completed' | 'failed'; + status: 'pending' | 'running' | 'completed' | 'failed' | 'cancelled'; totalPersons: number; analyzedPersons: number; candidatesFound: number; @@ -104,12 +167,68 @@ export interface DiscoveryProgress { // Store for tracking discovery runs and their results const MAX_STORED_RUNS = 100; +export const FULL_DISCOVERY_LIMITS = { + defaultBatchSize: 50, + maxBatchSize: 100, + defaultMaxPersons: 500, + maxPersons: 1000, +} as const; + +export interface FullDiscoveryOptions { + batchSize?: number; + maxPersons?: number; +} + +export interface NormalizedFullDiscoveryOptions { + batchSize: number; + maxPersons: number; +} + const discoveryRuns = new Map(); const discoveryResults = new Map(); +const activeDiscoveryRuns = new Map(); +let discoveryRunCounter = 0; + +function createDiscoveryRunId(dbId: string): string { + discoveryRunCounter += 1; + return `discovery-${dbId}-${Date.now()}-${discoveryRunCounter}`; +} + +function normalizeBoundedInteger( + value: unknown, + fallback: number, + maximum: number, + field: 'batchSize' | 'maxPersons', +): number { + if (value === undefined) return fallback; + if (typeof value !== 'number' || !Number.isInteger(value) || value < 1 || value > maximum) { + throw new DiscoveryInputError(`${field} must be a positive integer no greater than ${maximum}`); + } + return value; +} + +/** Validate request and programmatic full-discovery options before a run is created. */ +export function normalizeFullDiscoveryOptions(options?: FullDiscoveryOptions): NormalizedFullDiscoveryOptions { + return { + batchSize: normalizeBoundedInteger( + options?.batchSize, + FULL_DISCOVERY_LIMITS.defaultBatchSize, + FULL_DISCOVERY_LIMITS.maxBatchSize, + 'batchSize', + ), + maxPersons: normalizeBoundedInteger( + options?.maxPersons, + FULL_DISCOVERY_LIMITS.defaultMaxPersons, + FULL_DISCOVERY_LIMITS.maxPersons, + 'maxPersons', + ), + }; +} function evictOldestRun(): void { - if (discoveryRuns.size > MAX_STORED_RUNS) { - const oldestKey = discoveryRuns.keys().next().value; + if (discoveryRuns.size >= MAX_STORED_RUNS) { + const activeRunIds = new Set([...activeDiscoveryRuns.values()].map(({ runId }) => runId)); + const oldestKey = [...discoveryRuns.keys()].find(runId => !activeRunIds.has(runId)); if (oldestKey) { discoveryRuns.delete(oldestKey); discoveryResults.delete(oldestKey); @@ -209,16 +328,18 @@ export const aiDiscoveryService = { */ async startDiscovery( dbId: string, - options?: { - batchSize?: number; - maxPersons?: number; - } + options?: FullDiscoveryOptions, ): Promise<{ runId: string; message: string }> { - const runId = `discovery-${dbId}-${Date.now()}`; - const batchSize = options?.batchSize ?? 50; - const maxPersons = options?.maxPersons ?? 500; + const { batchSize, maxPersons } = normalizeFullDiscoveryOptions(options); + const activeRun = activeDiscoveryRuns.get(dbId); + if (activeRun) throw new DiscoveryRunConflictError(activeRun.runId); - // Initialize progress tracking (evict oldest if at capacity) + const runId = createDiscoveryRunId(dbId); + const controller = new AbortController(); + activeDiscoveryRuns.set(dbId, { runId, controller }); + + // Initialize progress tracking. Eviction never removes an active run, + // because its background operation must retain both progress and its guard. evictOldestRun(); discoveryRuns.set(runId, { status: 'pending', @@ -229,14 +350,26 @@ export const aiDiscoveryService = { totalBatches: 0, }); - // Run discovery asynchronously - this.runDiscovery(runId, dbId, batchSize, maxPersons).catch(err => { - const progress = discoveryRuns.get(runId); - if (progress) { + // Run discovery asynchronously. Always release the scope guard, including + // provider failures and cancellations, before a later request can start. + void this.runDiscovery(runId, dbId, batchSize, maxPersons, controller.signal) + .catch((err: Error) => { + const progress = discoveryRuns.get(runId); + if (!progress) return; + if (controller.signal.aborted || err instanceof DiscoveryCancelledError) { + progress.status = 'cancelled'; + logger.warn('ai-discovery', `Discovery ${runId} cancelled for dbId=${dbId}`); + return; + } progress.status = 'failed'; progress.error = err.message; - } - }); + logger.error('ai-discovery', `Discovery ${runId} failed for dbId=${dbId}: ${err.message}`); + }) + .finally(() => { + if (activeDiscoveryRuns.get(dbId)?.runId === runId) { + activeDiscoveryRuns.delete(dbId); + } + }); return { runId, message: 'Discovery started' }; }, @@ -248,6 +381,20 @@ export const aiDiscoveryService = { return discoveryRuns.get(runId) || null; }, + /** Request cancellation for the active full discovery in a database. */ + cancelDiscovery(dbId: string): { runId: string } | null { + const activeRun = activeDiscoveryRuns.get(dbId); + if (!activeRun) return null; + + logger.warn('ai-discovery', `Cancellation requested for discovery ${activeRun.runId} in dbId=${dbId}`); + activeRun.controller.abort(); + return { runId: activeRun.runId }; + }, + + getActiveDiscoveryRunId(dbId: string): string | null { + return activeDiscoveryRuns.get(dbId)?.runId ?? null; + }, + /** * Internal method to run discovery */ @@ -255,8 +402,10 @@ export const aiDiscoveryService = { runId: string, dbId: string, batchSize: number, - maxPersons: number + maxPersons: number, + signal?: AbortSignal, ): Promise { + throwIfCancelled(signal); const progress = discoveryRuns.get(runId); if (!progress) throw new Error('Run not found'); @@ -282,6 +431,7 @@ export const aiDiscoveryService = { // Process in batches for (let i = 0; i < personsToAnalyze.length; i += batchSize) { + throwIfCancelled(signal); progress.currentBatch = Math.floor(i / batchSize) + 1; const batchIds = personsToAnalyze.slice(i, i + batchSize); @@ -293,7 +443,8 @@ export const aiDiscoveryService = { const prompt = buildDiscoveryPrompt(batchSummaries, existingFavoriteIds); // Execute Claude CLI directly with piped input - const output = await executeAiPrompt(prompt, 300000); + const output = await executeAiPrompt(prompt, 300000, signal); + throwIfCancelled(signal); // Parse AI response const aiCandidates = parseAiResponse(output); diff --git a/tests/unit/services/aiDiscovery.spec.ts b/tests/unit/services/aiDiscovery.spec.ts new file mode 100644 index 00000000..47f39b81 --- /dev/null +++ b/tests/unit/services/aiDiscovery.spec.ts @@ -0,0 +1,258 @@ +import express from 'express'; +import request from 'supertest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const runner = { + createRun: vi.fn(), + executeApiRun: vi.fn(), + executeCliRun: vi.fn(), + stopRun: vi.fn(), +}; +const providers = { getActiveProvider: vi.fn() }; + +vi.mock('../../../server/src/services/database.service.js', () => ({ + databaseService: { getDatabase: vi.fn() }, +})); +vi.mock('../../../server/src/services/favorites.service.js', () => ({ + favoritesService: { getFavoritesInDatabase: vi.fn() }, + PRESET_TAGS: ['historical'], +})); +vi.mock('../../../server/src/services/id-mapping.service.js', () => ({ + idMappingService: { getExternalId: vi.fn() }, +})); +vi.mock('../../../server/src/db/sqlite.service.js', () => ({ sqliteService: {} })); +vi.mock('../../../server/src/services/ai-toolkit.service.js', () => ({ + getAIToolkit: () => ({ + services: { + providers, + runner, + }, + }), +})); +vi.mock('../../../server/src/lib/logger.js', () => ({ + logger: { start: vi.fn(), done: vi.fn(), error: vi.fn(), warn: vi.fn() }, +})); + +import { databaseService } from '../../../server/src/services/database.service.js'; +import { favoritesService } from '../../../server/src/services/favorites.service.js'; +import { + aiDiscoveryService, + DiscoveryInputError, + DiscoveryRunConflictError, + FULL_DISCOVERY_LIMITS, + normalizeFullDiscoveryOptions, +} from '../../../server/src/services/ai-discovery.service.js'; +import { aiDiscoveryRouter } from '../../../server/src/routes/ai-discovery.routes.js'; + +const db = { + 'PERSON-001': { name: 'Ada Example', lifespan: '1800-1870' }, + 'PERSON-002': { name: 'Bea Example', lifespan: '1820-1890' }, + 'PERSON-003': { name: 'Cy Example', lifespan: '1840-1910' }, +}; + +const completedProviderRun = () => { + runner.executeApiRun.mockImplementation((_runId, _provider, _model, _prompt, _workspace, _screenshots, onData, onComplete) => { + onData({ text: '[]' }); + onComplete({ success: true }); + return Promise.resolve('provider-run'); + }); +}; + +describe('full AI discovery', () => { + beforeEach(() => { + vi.clearAllMocks(); + runner.createRun.mockReset().mockResolvedValue({ + runId: `provider-run-${Date.now()}`, + provider: { type: 'api', defaultModel: 'test-model' }, + timeout: 300000, + }); + runner.executeCliRun.mockReset(); + runner.stopRun.mockReset().mockResolvedValue(true); + providers.getActiveProvider.mockReset().mockResolvedValue({ + id: 'test-provider', + name: 'Test provider', + type: 'api', + enabled: true, + defaultModel: 'test-model', + }); + completedProviderRun(); + vi.mocked(databaseService.getDatabase).mockResolvedValue(db as never); + vi.mocked(favoritesService.getFavoritesInDatabase).mockResolvedValue([]); + }); + + afterEach(async () => { + for (const dbId of ['batch-test', 'invalid-test', 'failure-test', 'cancel-test', 'rejection-test', 'cli-test', 'eviction-active-test']) { + aiDiscoveryService.cancelDiscovery(dbId); + } + }); + + it('uses safe defaults and rejects non-positive, fractional, non-numeric, and over-limit inputs', () => { + expect(normalizeFullDiscoveryOptions()).toEqual({ + batchSize: FULL_DISCOVERY_LIMITS.defaultBatchSize, + maxPersons: FULL_DISCOVERY_LIMITS.defaultMaxPersons, + }); + + for (const options of [ + { batchSize: 0 }, + { batchSize: -1 }, + { batchSize: 1.5 }, + { batchSize: '50' as unknown as number }, + { batchSize: FULL_DISCOVERY_LIMITS.maxBatchSize + 1 }, + { maxPersons: 0 }, + { maxPersons: -1 }, + { maxPersons: 1.5 }, + { maxPersons: '500' as unknown as number }, + { maxPersons: FULL_DISCOVERY_LIMITS.maxPersons + 1 }, + ]) { + expect(() => normalizeFullDiscoveryOptions(options)).toThrow(DiscoveryInputError); + } + }); + + it('does not create a provider run when programmatic callers pass invalid options', async () => { + await expect(aiDiscoveryService.startDiscovery('invalid-test', { batchSize: 0 })).rejects.toThrow(DiscoveryInputError); + expect(runner.createRun).not.toHaveBeenCalled(); + expect(runner.executeApiRun).not.toHaveBeenCalled(); + }); + + it('executes the expected number of bounded batches with a fake provider', async () => { + const { runId } = await aiDiscoveryService.startDiscovery('batch-test', { batchSize: 2, maxPersons: 3 }); + + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(runId)?.status).toBe('completed')); + + expect(runner.executeApiRun).toHaveBeenCalledTimes(2); + expect(aiDiscoveryService.getProgress(runId)).toMatchObject({ + totalPersons: 3, + totalBatches: 2, + analyzedPersons: 3, + }); + expect(aiDiscoveryService.getActiveDiscoveryRunId('batch-test')).toBeNull(); + + const restarted = await aiDiscoveryService.startDiscovery('batch-test', { batchSize: 3, maxPersons: 3 }); + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(restarted.runId)?.status).toBe('completed')); + expect(restarted.runId).not.toBe(runId); + }); + + it('retains an active run when completed history reaches its storage limit', async () => { + runner.executeApiRun.mockImplementationOnce(() => Promise.resolve('pending-provider-run')); + const active = await aiDiscoveryService.startDiscovery('eviction-active-test', { batchSize: 1, maxPersons: 1 }); + await vi.waitFor(() => expect(runner.executeApiRun).toHaveBeenCalled()); + + const completedRuns = await Promise.all( + Array.from({ length: 101 }, (_, index) => aiDiscoveryService.startDiscovery(`eviction-history-${index}`, { + batchSize: 1, + maxPersons: 1, + })), + ); + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(completedRuns.at(-1)!.runId)?.status).toBe('completed')); + + expect(aiDiscoveryService.getProgress(active.runId)?.status).toBe('running'); + expect(aiDiscoveryService.getActiveDiscoveryRunId('eviction-active-test')).toBe(active.runId); + }); + + it('rejects a duplicate start for the same database and releases the guard after provider failure', async () => { + runner.executeApiRun.mockImplementation((_runId, _provider, _model, _prompt, _workspace, _screenshots, _onData, onComplete) => { + onComplete({ success: false, error: 'provider unavailable' }); + return Promise.resolve('provider-run'); + }); + + const { runId } = await aiDiscoveryService.startDiscovery('failure-test', { batchSize: 1, maxPersons: 1 }); + await expect(aiDiscoveryService.startDiscovery('failure-test')).rejects.toThrow(DiscoveryRunConflictError); + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(runId)?.status).toBe('failed')); + await vi.waitFor(() => expect(aiDiscoveryService.getActiveDiscoveryRunId('failure-test')).toBeNull()); + }); + + it('cancels the in-flight provider run and releases the database guard', async () => { + runner.executeApiRun.mockImplementation(() => Promise.resolve('provider-run')); + + const { runId } = await aiDiscoveryService.startDiscovery('cancel-test', { batchSize: 1, maxPersons: 1 }); + await vi.waitFor(() => expect(runner.executeApiRun).toHaveBeenCalled()); + expect(aiDiscoveryService.cancelDiscovery('cancel-test')).toEqual({ runId }); + + await vi.waitFor(() => expect(runner.stopRun).toHaveBeenCalled()); + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(runId)?.status).toBe('cancelled')); + await vi.waitFor(() => expect(aiDiscoveryService.getActiveDiscoveryRunId('cancel-test')).toBeNull()); + }); + + it('releases the guard when the provider rejects before its completion callback', async () => { + runner.executeApiRun.mockRejectedValueOnce(new Error('provider transport error')); + + const { runId } = await aiDiscoveryService.startDiscovery('rejection-test', { batchSize: 1, maxPersons: 1 }); + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(runId)?.status).toBe('failed')); + await vi.waitFor(() => expect(aiDiscoveryService.getActiveDiscoveryRunId('rejection-test')).toBeNull()); + + const retry = await aiDiscoveryService.startDiscovery('rejection-test', { batchSize: 1, maxPersons: 1 }); + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(retry.runId)?.status).toBe('completed')); + }); + + it('accepts raw CLI output without mixing in provider metadata', async () => { + providers.getActiveProvider.mockResolvedValue({ + id: 'cli-provider', + name: 'CLI provider', + type: 'cli', + enabled: true, + defaultModel: 'test-model', + }); + runner.createRun.mockResolvedValue({ + runId: 'cli-run', + provider: { type: 'cli', defaultModel: 'test-model' }, + timeout: 300000, + }); + runner.executeCliRun.mockImplementation((_runId, _provider, _prompt, _workspace, onData, onComplete) => { + onData('[]'); + onComplete({ success: true }); + return Promise.resolve('cli-run'); + }); + + const { runId } = await aiDiscoveryService.startDiscovery('cli-test', { batchSize: 1, maxPersons: 1 }); + await vi.waitFor(() => expect(aiDiscoveryService.getProgress(runId)?.status).toBe('completed')); + expect(runner.executeCliRun).toHaveBeenCalledOnce(); + }); +}); + +describe('full AI discovery route', () => { + const app = express(); + app.use(express.json()); + app.use('/api/ai-discovery', aiDiscoveryRouter); + + afterEach(() => vi.restoreAllMocks()); + + it.each([ + [{ batchSize: 0 }], + [{ batchSize: -1 }], + [{ batchSize: 1.5 }], + [{ batchSize: '50' }], + [{ maxPersons: 0 }], + [{ maxPersons: FULL_DISCOVERY_LIMITS.maxPersons + 1 }], + ])('returns 400 without starting a run for invalid options: %j', async (body) => { + const start = vi.spyOn(aiDiscoveryService, 'startDiscovery'); + + const response = await request(app).post('/api/ai-discovery/route-test/start').send(body).expect(400); + + expect(response.body.success).toBe(false); + expect(start).not.toHaveBeenCalled(); + }); + + it('returns the active run id when a concurrent start conflicts', async () => { + vi.spyOn(aiDiscoveryService, 'startDiscovery').mockRejectedValue(new DiscoveryRunConflictError('discovery-route-test')); + + const response = await request(app).post('/api/ai-discovery/route-test/start').send({}).expect(409); + + expect(response.body).toMatchObject({ success: false, data: { runId: 'discovery-route-test' } }); + }); + + it('requests cancellation through the database-scoped route', async () => { + vi.spyOn(aiDiscoveryService, 'cancelDiscovery').mockReturnValue({ runId: 'discovery-route-test' }); + + const response = await request(app).post('/api/ai-discovery/route-test/cancel').expect(200); + + expect(response.body).toMatchObject({ success: true, data: { runId: 'discovery-route-test' } }); + }); + + it('returns 404 when there is no active discovery to cancel', async () => { + vi.spyOn(aiDiscoveryService, 'cancelDiscovery').mockReturnValue(null); + + const response = await request(app).post('/api/ai-discovery/route-test/cancel').expect(404); + + expect(response.body.success).toBe(false); + }); +});