diff --git a/backend/src/middleware/__tests__/loginRateLimit.test.ts b/backend/src/middleware/__tests__/loginRateLimit.test.ts index 6a8f07f8..a3f99663 100644 --- a/backend/src/middleware/__tests__/loginRateLimit.test.ts +++ b/backend/src/middleware/__tests__/loginRateLimit.test.ts @@ -1,4 +1,6 @@ +import type { NextFunction, Request, Response } from 'express'; import { beforeEach, describe, expect, it, vi } from 'vitest'; + import { clearLoginFailures, loginRateLimit, @@ -7,10 +9,18 @@ import { } from '../loginRateLimit.js'; function mockRes() { - const res: any = {}; - res.status = vi.fn().mockReturnValue(res); - res.json = vi.fn().mockReturnValue(res); - return res; + const res = { + status: vi.fn().mockReturnThis(), + json: vi.fn().mockReturnThis(), + }; + return res as unknown as Response & { + status: ReturnType; + json: ReturnType; + }; +} + +function mockReq(ip: string, email: string): Request { + return { ip, body: { email }, socket: {} } as unknown as Request; } describe('loginRateLimit', () => { @@ -23,9 +33,9 @@ describe('loginRateLimit', () => { }); it('allows requests under the limit', () => { - const req: any = { ip: '1.1.1.1', body: { email: 'a@x.com' }, socket: {} }; + const req = mockReq('1.1.1.1', 'a@x.com'); const res = mockRes(); - const next = vi.fn(); + const next = vi.fn() as unknown as NextFunction; loginRateLimit(req, res, next); expect(next).toHaveBeenCalled(); }); @@ -33,18 +43,10 @@ describe('loginRateLimit', () => { it('429s after per-email max', () => { const res = mockRes(); for (let i = 0; i < 2; i++) { - loginRateLimit( - { ip: '2.2.2.2', body: { email: 'b@x.com' }, socket: {} } as any, - mockRes(), - vi.fn() - ); + loginRateLimit(mockReq('2.2.2.2', 'b@x.com'), mockRes(), vi.fn() as unknown as NextFunction); } - const next = vi.fn(); - loginRateLimit( - { ip: '2.2.2.2', body: { email: 'b@x.com' }, socket: {} } as any, - res, - next - ); + const next = vi.fn() as unknown as NextFunction; + loginRateLimit(mockReq('2.2.2.2', 'b@x.com'), res, next); expect(res.status).toHaveBeenCalledWith(429); expect(next).not.toHaveBeenCalled(); }); @@ -54,12 +56,8 @@ describe('loginRateLimit', () => { recordLoginFailure('c@x.com', '3.3.3.3'); recordLoginFailure('c@x.com', '3.3.3.3'); const res = mockRes(); - const next = vi.fn(); - loginRateLimit( - { ip: '3.3.3.3', body: { email: 'c@x.com' }, socket: {} } as any, - res, - next - ); + const next = vi.fn() as unknown as NextFunction; + loginRateLimit(mockReq('3.3.3.3', 'c@x.com'), res, next); expect(res.status).toHaveBeenCalledWith(429); clearLoginFailures('c@x.com', '3.3.3.3'); }); diff --git a/backend/src/middleware/__tests__/requireDeploymentAuth.test.ts b/backend/src/middleware/__tests__/requireDeploymentAuth.test.ts index 859d8736..11f1c8af 100644 --- a/backend/src/middleware/__tests__/requireDeploymentAuth.test.ts +++ b/backend/src/middleware/__tests__/requireDeploymentAuth.test.ts @@ -1,7 +1,10 @@ -import { describe, expect, it, vi, beforeEach } from 'vitest'; +import type { NextFunction, Response } from 'express'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; -const getById = vi.fn(); -const verifyApiKey = vi.fn(); +const { getById, verifyApiKey } = vi.hoisted(() => ({ + getById: vi.fn(), + verifyApiKey: vi.fn(), +})); vi.mock('../../repositories/deploymentRepository.js', () => ({ createDeploymentRepository: () => ({ getById }), @@ -20,13 +23,22 @@ vi.mock('../resourceOwnership.js', () => ({ verifyProjectOwnership: vi.fn(), })); +import type { PredictRequest } from '../requireDeploymentAuth.js'; import { requireDeploymentAuth } from '../requireDeploymentAuth.js'; function mockRes() { - const res: any = {}; - res.status = vi.fn().mockReturnValue(res); - res.json = vi.fn().mockReturnValue(res); - return res; + const res = { + status: vi.fn().mockReturnThis(), + json: vi.fn().mockReturnThis(), + }; + return res as unknown as Response & { + status: ReturnType; + json: ReturnType; + }; +} + +function mockReq(params: Record, headers: Record = {}): PredictRequest { + return { params, headers } as unknown as PredictRequest; } describe('requireDeploymentAuth enumeration resistance', () => { @@ -37,9 +49,9 @@ describe('requireDeploymentAuth enumeration resistance', () => { it('returns 404 Not found for missing deployment', async () => { getById.mockResolvedValue(null); - const req: any = { params: { deploymentId: 'missing' }, headers: {} }; + const req = mockReq({ deploymentId: 'missing' }); const res = mockRes(); - const next = vi.fn(); + const next = vi.fn() as unknown as NextFunction; await requireDeploymentAuth(req, res, next); expect(res.status).toHaveBeenCalledWith(404); expect(res.json).toHaveBeenCalledWith({ error: 'Not found' }); @@ -49,12 +61,9 @@ describe('requireDeploymentAuth enumeration resistance', () => { it('returns the same 404 Not found for a bad API key', async () => { getById.mockResolvedValue({ deploymentId: 'dep-1', projectId: 'p1' }); verifyApiKey.mockResolvedValue(null); - const req: any = { - params: { deploymentId: 'dep-1' }, - headers: { 'x-api-key': 'bad-key' }, - }; + const req = mockReq({ deploymentId: 'dep-1' }, { 'x-api-key': 'bad-key' }); const res = mockRes(); - const next = vi.fn(); + const next = vi.fn() as unknown as NextFunction; await requireDeploymentAuth(req, res, next); expect(res.status).toHaveBeenCalledWith(404); expect(res.json).toHaveBeenCalledWith({ error: 'Not found' }); diff --git a/backend/src/routes/auth.ts b/backend/src/routes/auth.ts index 3a5f565c..f2a49c97 100644 --- a/backend/src/routes/auth.ts +++ b/backend/src/routes/auth.ts @@ -5,8 +5,8 @@ import { z } from 'zod'; import { appLogger } from '../logging/logger.js'; import { asyncHandler } from '../middleware/asyncHandler.js'; import { requireAuth, requireAuthAllowUnverified, invalidateUserCache } from '../middleware/auth.js'; -import { validateRequest } from '../middleware/validateRequest.js'; import { loginRateLimit, recordLoginFailure, clearLoginFailures } from '../middleware/loginRateLimit.js'; +import { validateRequest } from '../middleware/validateRequest.js'; import { UserRepository } from '../repositories/userRepository.js'; import { authService } from '../services/authService.js'; import { emailService } from '../services/emailService.js'; diff --git a/backend/src/routes/auth/__tests__/oauthHandler.test.ts b/backend/src/routes/auth/__tests__/oauthHandler.test.ts index d3d9f1ee..c7c5bb3c 100644 --- a/backend/src/routes/auth/__tests__/oauthHandler.test.ts +++ b/backend/src/routes/auth/__tests__/oauthHandler.test.ts @@ -1,5 +1,20 @@ +import type { Request, Response } from 'express'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +const { + hashPassword, + generatePasswordResetToken, + generateTokens, + hashRefreshToken, + refreshTokenExpiryMs, +} = vi.hoisted(() => ({ + hashPassword: vi.fn().mockResolvedValue('hash'), + generatePasswordResetToken: vi.fn().mockReturnValue('rand'), + generateTokens: vi.fn().mockReturnValue({ accessToken: 'a', refreshToken: 'r' }), + hashRefreshToken: vi.fn().mockReturnValue('rh'), + refreshTokenExpiryMs: vi.fn().mockReturnValue(1000), +})); + vi.mock('../../../config.js', () => ({ env: { googleClientId: 'cid', @@ -12,12 +27,6 @@ vi.mock('../../../logging/logger.js', () => ({ appLogger: { error: vi.fn(), warn: vi.fn(), info: vi.fn() }, })); -const hashPassword = vi.fn().mockResolvedValue('hash'); -const generatePasswordResetToken = vi.fn().mockReturnValue('rand'); -const generateTokens = vi.fn().mockReturnValue({ accessToken: 'a', refreshToken: 'r' }); -const hashRefreshToken = vi.fn().mockReturnValue('rh'); -const refreshTokenExpiryMs = vi.fn().mockReturnValue(1000); - vi.mock('../../../services/authService.js', () => ({ authService: { hashPassword, @@ -28,58 +37,81 @@ vi.mock('../../../services/authService.js', () => ({ }, })); +import type { UserRepository } from '../../../repositories/userRepository.js'; import { handleGoogleCallback } from '../oauthHandler.js'; +type GoogleUser = { + id: string; + email: string; + name: string; + verified_email: boolean; +}; + +const googleUserStore: { current: GoogleUser | null } = { current: null }; + function mockRes() { - const res: any = {}; - res.status = vi.fn().mockReturnValue(res); - res.json = vi.fn().mockReturnValue(res); - return res; + const res = { + status: vi.fn().mockReturnThis(), + json: vi.fn().mockReturnThis(), + }; + return res as unknown as Response & { + status: ReturnType; + json: ReturnType; + }; } -function mockRepo(overrides: Record = {}) { +function mockRepo(overrides: Partial> = {}): UserRepository { return { findByEmail: vi.fn().mockResolvedValue(null), create: vi.fn(), markEmailVerified: vi.fn(), findById: vi.fn(), updateLastLogin: vi.fn(), - toSafeUser: vi.fn((u) => u), + toSafeUser: vi.fn((u: unknown) => u), storeRefreshToken: vi.fn(), ...overrides, - } as any; + } as unknown as UserRepository; +} + +function mockReq(): Request { + return { body: { code: 'c' }, ip: '1', get: () => 'ua' } as unknown as Request; +} + +function mockFetchResponse(body: unknown): globalThis.Response { + return { + ok: true, + json: async () => body, + } as unknown as globalThis.Response; } describe('handleGoogleCallback', () => { beforeEach(() => { + googleUserStore.current = null; vi.stubGlobal( 'fetch', - vi.fn(async (url: string) => { + vi.fn(async (url: string | URL | globalThis.Request) => { if (String(url).includes('oauth2.googleapis.com/token')) { - return { ok: true, json: async () => ({ access_token: 'tok', id_token: 'id' }) } as any; + return mockFetchResponse({ access_token: 'tok', id_token: 'id' }); } - return { - ok: true, - json: async () => (globalThis as any).__googleUser, - } as any; + return mockFetchResponse(googleUserStore.current); }) ); }); it('rejects unverified Google emails', async () => { - (globalThis as any).__googleUser = { + googleUserStore.current = { id: 'g1', email: 'a@x.com', name: 'A', verified_email: false, }; const res = mockRes(); - await handleGoogleCallback({ body: { code: 'c' }, ip: '1', get: () => 'ua' } as any, res, mockRepo()); + await handleGoogleCallback(mockReq(), res, mockRepo()); expect(res.status).toHaveBeenCalledWith(403); }); it('blocks silent merge into unverified password account', async () => { - (globalThis as any).__googleUser = { + googleUserStore.current = { id: 'g1', email: 'a@x.com', name: 'A', @@ -88,7 +120,7 @@ describe('handleGoogleCallback', () => { const existing = { user_id: 'u1', email: 'a@x.com', email_verified: false }; const res = mockRes(); await handleGoogleCallback( - { body: { code: 'c' }, ip: '1', get: () => 'ua' } as any, + mockReq(), res, mockRepo({ findByEmail: vi.fn().mockResolvedValue(existing) }) ); @@ -96,7 +128,7 @@ describe('handleGoogleCallback', () => { }); it('logs into verified existing account', async () => { - (globalThis as any).__googleUser = { + googleUserStore.current = { id: 'g1', email: 'a@x.com', name: 'A', @@ -108,7 +140,7 @@ describe('handleGoogleCallback', () => { toSafeUser: vi.fn().mockReturnValue(existing), }); const res = mockRes(); - await handleGoogleCallback({ body: { code: 'c' }, ip: '1', get: () => 'ua' } as any, res, repo); + await handleGoogleCallback(mockReq(), res, repo); expect(repo.updateLastLogin).toHaveBeenCalledWith('u1'); expect(res.json).toHaveBeenCalled(); expect(res.status).not.toHaveBeenCalledWith(409); diff --git a/backend/src/routes/datasets.ts b/backend/src/routes/datasets.ts index d73b54a6..ab804d52 100644 --- a/backend/src/routes/datasets.ts +++ b/backend/src/routes/datasets.ts @@ -13,8 +13,8 @@ import { getProjectRepository } from '../repositories/projectRepository.js'; import { resolveDatasetTableName } from '../services/datasetLoader.js'; import { ensureProjectDatasetSqlNames, resolveDatasetSqlName } from '../services/datasetSqlNames.js'; import { getDatasetQueryState, rebuildDatasetTableFromSource } from '../services/datasetTableManager.js'; -import { getWorkflowRepository } from '../services/workflows/repository/index.js'; import * as notebookService from '../services/notebook/notebookService.js'; +import { getWorkflowRepository } from '../services/workflows/repository/index.js'; import type { AuthRequest } from '../types/auth.js'; import { getErrorMessage, sendNotFound } from '../utils/errors.js'; import { getDatasetPath } from '../utils/pathUtils.js'; diff --git a/backend/src/routes/notebooks/notebookRoutes.ts b/backend/src/routes/notebooks/notebookRoutes.ts index 87e6da9e..f29f9484 100644 --- a/backend/src/routes/notebooks/notebookRoutes.ts +++ b/backend/src/routes/notebooks/notebookRoutes.ts @@ -6,11 +6,11 @@ import { verifyProjectOwnership } from '../../middleware/resourceOwnership.js'; import { getProjectRepository } from '../../repositories/projectRepository.js'; import * as kernelManager from '../../services/kernelManager.js'; import { getOrEnsureContainer } from '../../services/notebook/cellExecutionService.js'; -import * as notebookService from '../../services/notebook/notebookService.js'; import { getNotebookRecoveryCandidate, recoverNotebookFromWorkflowHistory } from '../../services/notebook/notebookRecoveryService.js'; +import * as notebookService from '../../services/notebook/notebookService.js'; import type { AuthRequest } from '../../types/auth.js'; import { NotebookKindSchema } from '../../types/notebook.js'; diff --git a/backend/src/services/llm/prompts/trainingWorkflow.ts b/backend/src/services/llm/prompts/trainingWorkflow.ts index 74ac699d..78a671d1 100644 --- a/backend/src/services/llm/prompts/trainingWorkflow.ts +++ b/backend/src/services/llm/prompts/trainingWorkflow.ts @@ -4,8 +4,8 @@ import type { DatasetProfile } from '../../../types/dataset.js'; import type { ToolResult } from '../../../types/llm.js'; -import type { FeatureSpec } from '../../featureEngineering.js'; import { findLikelyIdentifierColumns } from '../../columnClassification.js'; +import type { FeatureSpec } from '../../featureEngineering.js'; import { buildTemplateSummary } from '../../modelTemplates.js'; import type { LlmRequest, diff --git a/backend/src/services/llm/trainingTools/executionTools.ts b/backend/src/services/llm/trainingTools/executionTools.ts index dfa8cef9..f2f6921b 100644 --- a/backend/src/services/llm/trainingTools/executionTools.ts +++ b/backend/src/services/llm/trainingTools/executionTools.ts @@ -1,10 +1,11 @@ import { appLogger } from '../../../logging/logger.js'; import { executeMcpTool } from '../../mcp/mcpAdapter.js'; import { nowIso } from '../preprocessingTools/helpers.js'; -import { normalizeWorkflowPrepSegments } from './workflowPrepSegments.js'; + import { resolveExperiment } from './types.js'; import type { TrainingToolContext, TrainingToolHandler, TrainingToolResult } from './types.js'; +import { normalizeWorkflowPrepSegments } from './workflowPrepSegments.js'; function normalizeMetricsRecord(metrics: unknown): Record { if (!metrics || typeof metrics !== 'object' || Array.isArray(metrics)) { diff --git a/backend/src/services/modelTraining.ts b/backend/src/services/modelTraining.ts index 3bce7f08..be1b5e98 100644 --- a/backend/src/services/modelTraining.ts +++ b/backend/src/services/modelTraining.ts @@ -15,8 +15,8 @@ import { syncWorkspaceDatasets } from './executionWorkspace.js'; import * as kernelManager from './kernelManager.js'; import { getModelTemplate, listModelTemplates, resolveModelTemplateId } from './modelTemplates.js'; import { DEFAULT_MODEL_TEST_SIZE, normalizeModelTestSize } from './modelTestSize.js'; -import { deleteTuningStudiesByModelId } from './tuningService.js'; import { resolveContainerSafeNJobs } from './parallelism.js'; +import { deleteTuningStudiesByModelId } from './tuningService.js'; const datasetRepository = createDatasetRepository(env.datasetMetadataPath); const modelRepository = createModelRepository(env.modelMetadataPath); diff --git a/backend/src/services/notebook/notebookRecoveryService.ts b/backend/src/services/notebook/notebookRecoveryService.ts index 5487e1df..ae303a46 100644 --- a/backend/src/services/notebook/notebookRecoveryService.ts +++ b/backend/src/services/notebook/notebookRecoveryService.ts @@ -1,8 +1,9 @@ +import * as notebookRepo from '../../repositories/notebookRepository.js'; import type { CellOutput, CellStatus } from '../../types/notebook.js'; -import type { WorkflowPhase, WorkflowRunSnapshot } from '../workflows/types.js'; import { getWorkflowRepository } from '../workflows/repository/index.js'; +import type { WorkflowPhase, WorkflowRunSnapshot } from '../workflows/types.js'; + import * as notebookService from './notebookService.js'; -import * as notebookRepo from '../../repositories/notebookRepository.js'; type RecoverablePhase = 'preprocessing' | 'feature-engineering' | 'training'; diff --git a/backend/src/services/parallelism.test.ts b/backend/src/services/parallelism.test.ts index 168b5566..5d2afc92 100644 --- a/backend/src/services/parallelism.test.ts +++ b/backend/src/services/parallelism.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; + import { buildNJobsPythonSnippet, resolveContainerSafeNJobs } from './parallelism.js'; describe('resolveContainerSafeNJobs', () => { diff --git a/backend/src/services/tuningService.ts b/backend/src/services/tuningService.ts index 6489b17b..4a5efdf3 100644 --- a/backend/src/services/tuningService.ts +++ b/backend/src/services/tuningService.ts @@ -26,8 +26,8 @@ import { resolveAndHealTargetColumn } from '../utils/modelUtils.js'; import { getModelTemplate } from './modelTemplates.js'; import { resolveModelTestSize } from './modelTestSize.js'; -import { import { buildNJobsPythonSnippet } from './parallelism.js'; +import { buildOutputDirSetup, buildResultSaving, buildStandardImports, diff --git a/frontend/src/lib/api/client.ts b/frontend/src/lib/api/client.ts index 27a854d4..f5146abb 100644 --- a/frontend/src/lib/api/client.ts +++ b/frontend/src/lib/api/client.ts @@ -263,6 +263,10 @@ export async function apiFetch(path: string, options: RequestOptions = {}): Prom await applyAuthSideEffects(response); + if (response.ok) { + notifyApiActivity(); + } + return response; } diff --git a/frontend/src/stores/__tests__/deploymentStore.test.ts b/frontend/src/stores/__tests__/deploymentStore.test.ts index bb3d20e8..9e0b54d8 100644 --- a/frontend/src/stores/__tests__/deploymentStore.test.ts +++ b/frontend/src/stores/__tests__/deploymentStore.test.ts @@ -44,25 +44,56 @@ describe('deploymentStore.deploy', () => { vi.clearAllMocks(); }); - it('hydrates persisted failed deployments after createDeployment rejects', async () => { - const failedDeployment = buildDeployment({ + it('inserts an optimistic failed deployment when createDeployment rejects', async () => { + createDeploymentMock.mockRejectedValueOnce(new Error('Inference container exited with code 3')); + // Create never persisted — server list empty; optimistic row must remain. + listDeploymentsMock.mockResolvedValueOnce({ deployments: [] }); + + await expect( + useDeploymentStore.getState().deploy('model-1', 'project-1', 'Endpoint 1'), + ).rejects.toThrow('Inference container exited with code 3'); + + // Allow the fire-and-forget reconcile microtask to finish. + await Promise.resolve(); + await Promise.resolve(); + + expect(listDeploymentsMock).toHaveBeenCalledWith('project-1'); + + const state = useDeploymentStore.getState(); + expect(state.deployments).toHaveLength(1); + const failed = state.deployments[0]; + expect(failed.status).toBe('failed'); + expect(failed.modelId).toBe('model-1'); + expect(failed.projectId).toBe('project-1'); + expect(failed.name).toBe('Endpoint 1'); + expect(failed.errorMessage).toBe('Inference container exited with code 3'); + expect(failed.deploymentId).toMatch(/^failed-\d+$/); + expect(state.selectedDeploymentId).toBe(failed.deploymentId); + expect(state.error).toBe('Inference container exited with code 3'); + expect(state.isLoading).toBe(false); + }); + + it('prefers a server-persisted failed deployment over the optimistic row', async () => { + const persisted = buildDeployment({ deploymentId: 'dep-failed', status: 'failed', errorMessage: 'Inference container exited with code 3', }); createDeploymentMock.mockRejectedValueOnce(new Error('Inference container exited with code 3')); - listDeploymentsMock.mockResolvedValueOnce({ deployments: [failedDeployment] }); + listDeploymentsMock.mockResolvedValueOnce({ deployments: [persisted] }); await expect( useDeploymentStore.getState().deploy('model-1', 'project-1', 'Endpoint 1'), ).rejects.toThrow('Inference container exited with code 3'); - expect(listDeploymentsMock).toHaveBeenCalledWith('project-1'); - expect(useDeploymentStore.getState().deployments).toEqual([failedDeployment]); - expect(useDeploymentStore.getState().selectedDeploymentId).toBe('dep-failed'); - expect(useDeploymentStore.getState().error).toBe('Inference container exited with code 3'); - expect(useDeploymentStore.getState().isLoading).toBe(false); + await Promise.resolve(); + await Promise.resolve(); + + const state = useDeploymentStore.getState(); + expect(state.deployments).toEqual([persisted]); + expect(state.selectedDeploymentId).toBe('dep-failed'); + expect(state.error).toBe('Inference container exited with code 3'); }); it('stores successful deployments without an extra refresh', async () => { diff --git a/frontend/src/stores/deploymentStore.ts b/frontend/src/stores/deploymentStore.ts index f57d4d29..c02c3a26 100644 --- a/frontend/src/stores/deploymentStore.ts +++ b/frontend/src/stores/deploymentStore.ts @@ -73,8 +73,37 @@ export const useDeploymentStore = create((set, get) => ({ error, isLoading: false, })); - // Best-effort reconcile with server (may no-op if create never persisted). - void get().refreshDeployments(projectId).catch(() => undefined); + // Best-effort reconcile: if the server persisted a failed row, prefer it. + // Do not call refreshDeployments here — it clears `error` and would wipe + // the optimistic row when create never persisted. + void api + .listDeployments(projectId) + .then(({ deployments: serverDeployments }) => { + if (serverDeployments.length === 0) return; + set(state => { + const serverIds = new Set(serverDeployments.map(d => d.deploymentId)); + // Drop optimistic rows that the server has already persisted (same model + name). + const keepOptimistic = state.deployments.filter(d => { + if (!d.deploymentId.startsWith('failed-') || serverIds.has(d.deploymentId)) { + return false; + } + return !serverDeployments.some( + s => s.modelId === d.modelId && s.name === d.name, + ); + }); + const deployments = [...serverDeployments, ...keepOptimistic]; + const selectedStillPresent = deployments.some( + d => d.deploymentId === state.selectedDeploymentId, + ); + return { + deployments, + selectedDeploymentId: selectedStillPresent + ? state.selectedDeploymentId + : (serverDeployments[0]?.deploymentId ?? state.selectedDeploymentId), + }; + }); + }) + .catch(() => undefined); throw err; } },