From 2b35449e46768d5c9864d80a17e1cae50a3584c8 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Fri, 26 Jun 2026 01:28:36 -0700 Subject: [PATCH] refactor: consolidate duplicated cross-cutting constants Introduce a dependency-free src/lib/constants.ts as the single source of truth for constants that are duplicated or span the client/server boundary, and replace the scattered magic numbers with named imports: - AUDIT_LOG_FETCH_LIMIT (1000): the audit-log fetch limit, used by both the client request (useQuery) and the server-side clamp (query-service). These previously drifted (client 1000 vs server 500); now they can't. - AUDIT_LOG_DEFAULT_LIMIT (100): server default when no limit is given. - LIVE_QUERY_REFETCH_INTERVAL_MS (5000): polling cadence shared by the active-sessions and audit-log queries (was inlined 3x). - MCP_MAX_RESULT_ROWS (1000) / MCP_CATALOG_PAGE_SIZE (100): MCP row cap and catalog page size (mcp.ts keeps its local MAX_RESULT_ROWS/PAGE_SIZE names, now sourced from the shared module, so importers/tests are intact). Scope is intentionally limited to duplicated / cross-tier / contract constants; well-named feature-local constants (panel widths, breakpoints, retry counts, etc.) are left next to their feature. Co-Authored-By: Claude Opus 4.8 --- server/mcp.ts | 5 ++-- server/services/query-service.ts | 5 ++-- src/hooks/useQuery.ts | 13 +++++------ src/lib/constants.ts | 39 ++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 src/lib/constants.ts diff --git a/server/mcp.ts b/server/mcp.ts index 65502ff..c125030 100644 --- a/server/mcp.ts +++ b/server/mcp.ts @@ -17,17 +17,18 @@ import { getAgentPermissions, type Permission } from './lib/iam' import { detectRequiredPermissions } from './lib/sql-permissions' import { buildExecutableSql, formatExecutionError } from './lib/execute-sql' import { auditSQL } from './lib/audit' +import { MCP_CATALOG_PAGE_SIZE, MCP_MAX_RESULT_ROWS } from '../src/lib/constants' declare const __APP_VERSION__: string export const MCP_PATH = '/mcp' -const PAGE_SIZE = 100 +const PAGE_SIZE = MCP_CATALOG_PAGE_SIZE // Hard cap on rows returned by the execution tools, so a broad SELECT can't flood an agent's // context. The full result is fetched, then capped; a `truncated` flag tells the agent to narrow // (LIMIT/WHERE or a smaller maxRows). MCP only — the UI route is intentionally uncapped because it // needs the full result set for CSV export and inline editing. -export const MAX_RESULT_ROWS = 1000 +export const MAX_RESULT_ROWS = MCP_MAX_RESULT_ROWS // A resolved MCP caller — identity + audit actor for one agent. Permission resolution // itself lives in iam.ts (getAgentPermissions), the single home for that decision. diff --git a/server/services/query-service.ts b/server/services/query-service.ts index d31fc1d..9c17e26 100644 --- a/server/services/query-service.ts +++ b/server/services/query-service.ts @@ -9,6 +9,7 @@ import { hasPermission, requirePermission, requirePermissions, requireAnyPermiss import { detectRequiredPermissions } from "../lib/sql-permissions"; import { buildExecutableSql, formatExecutionError } from "../lib/execute-sql"; import { auditSQL, auditExport, listAuditEvents, listSystemAuditEvents, type AuditEvent } from "../lib/audit"; +import { AUDIT_LOG_FETCH_LIMIT, AUDIT_LOG_DEFAULT_LIMIT } from "../../src/lib/constants"; // Track active queries by queryId -> { pid, connectionDetails, email } const activeQueries = new Map(); @@ -1223,7 +1224,7 @@ export const queryServiceHandlers: ServiceImpl = { requirePermission(user, req.connectionId, 'admin', 'view audit log'); getConnectionDetails(req.connectionId); - const limit = req.limit > 0 ? Math.min(req.limit, 1000) : 100; + const limit = req.limit > 0 ? Math.min(req.limit, AUDIT_LOG_FETCH_LIMIT) : AUDIT_LOG_DEFAULT_LIMIT; const entries = listAuditEvents(req.connectionId, limit).map(toAuditLogEntry); return { entries }; @@ -1241,7 +1242,7 @@ export const queryServiceHandlers: ServiceImpl = { throw new ConnectError("Permission denied: viewing the system audit log requires instance owner", Code.PermissionDenied); } - const limit = req.limit > 0 ? Math.min(req.limit, 1000) : 100; + const limit = req.limit > 0 ? Math.min(req.limit, AUDIT_LOG_FETCH_LIMIT) : AUDIT_LOG_DEFAULT_LIMIT; const entries = listSystemAuditEvents(limit).map(toAuditLogEntry); return { entries }; diff --git a/src/hooks/useQuery.ts b/src/hooks/useQuery.ts index c77c150..7e76e2b 100644 --- a/src/hooks/useQuery.ts +++ b/src/hooks/useQuery.ts @@ -1,5 +1,6 @@ import { useMutation, useQuery, useQueryClient, type QueryClient } from '@tanstack/react-query'; import { queryClient, connectionClient, aiClient } from '../lib/connect-client'; +import { AUDIT_LOG_FETCH_LIMIT, LIVE_QUERY_REFETCH_INTERVAL_MS } from '../lib/constants'; import type { ColumnMetadata } from '../components/sql-editor/hooks/useEditorTabs'; // Query keys @@ -309,7 +310,7 @@ export function useActiveProcesses(connectionId: string, enabled = true) { return response.sessions; }, enabled: enabled && !!connectionId, - refetchInterval: 5000, + refetchInterval: LIVE_QUERY_REFETCH_INTERVAL_MS, }); } @@ -328,17 +329,15 @@ export function useTerminateProcess() { }); } -const AUDIT_LOG_LIMIT = 1000; - export function useAuditLogEntries(connectionId: string, enabled = true) { return useQuery({ queryKey: queryKeys.auditLog(connectionId), queryFn: async () => { - const response = await queryClient.getAuditLogEntries({ connectionId, limit: AUDIT_LOG_LIMIT }); + const response = await queryClient.getAuditLogEntries({ connectionId, limit: AUDIT_LOG_FETCH_LIMIT }); return response.entries; }, enabled: enabled && !!connectionId, - refetchInterval: 5000, + refetchInterval: LIVE_QUERY_REFETCH_INTERVAL_MS, }); } @@ -348,11 +347,11 @@ export function useSystemAuditLogEntries(enabled = true) { return useQuery({ queryKey: queryKeys.systemAuditLog(), queryFn: async () => { - const response = await queryClient.getSystemAuditLogEntries({ limit: AUDIT_LOG_LIMIT }); + const response = await queryClient.getSystemAuditLogEntries({ limit: AUDIT_LOG_FETCH_LIMIT }); return response.entries; }, enabled, - refetchInterval: 5000, + refetchInterval: LIVE_QUERY_REFETCH_INTERVAL_MS, }); } diff --git a/src/lib/constants.ts b/src/lib/constants.ts new file mode 100644 index 0000000..7d6af2c --- /dev/null +++ b/src/lib/constants.ts @@ -0,0 +1,39 @@ +// Centralized home for cross-cutting and otherwise-duplicated constants. +// +// Keep this file dependency-free (pure values only) so both the client and the +// server can import it without dragging browser- or server-only code across the +// boundary. Feature-local constants that are used in exactly one place and are +// already well-named should stay next to their feature — only values that are +// duplicated, span the client/server boundary, or define a runtime contract +// belong here. + +// --- Audit log --- + +/** + * Max audit-log rows fetched per tab, and the server-side hard clamp on the + * `limit` request field. The client request and the server clamp must agree, so + * they share this single source of truth. + */ +export const AUDIT_LOG_FETCH_LIMIT = 1000 + +/** Server default applied when an audit-log request omits a positive `limit`. */ +export const AUDIT_LOG_DEFAULT_LIMIT = 100 + +// --- Live polling --- + +/** + * Refetch cadence (ms) for client queries that poll live data — active sessions + * and the audit-log tabs. + */ +export const LIVE_QUERY_REFETCH_INTERVAL_MS = 5000 + +// --- MCP server --- + +/** + * Hard cap on rows an MCP execution tool returns. The full result is fetched, + * then capped; when capped the response sets `truncated` so the agent can narrow. + */ +export const MCP_MAX_RESULT_ROWS = 1000 + +/** Page size for MCP catalog browsing (`list_objects` pagination). */ +export const MCP_CATALOG_PAGE_SIZE = 100