From 72afecffebc6c259a2d12372fe7e6a3f8963d1ff Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Sat, 16 May 2026 00:27:20 -0300 Subject: [PATCH] fix(migrations): resolve version collisions and add batch deletion API (#2294) - Rename 056_provider_connection_quota_window_thresholds.sql to 057 - Add LEGACY_VERSION_SLOT_MIGRATIONS entries for backward compatibility - Add deleteBatch/deleteCompletedBatches to batches.ts - Add DELETE routes for batches (single + bulk) - Add batch deletion buttons to dashboard - Broaden dashboard session auth to all client API routes - Add quota_window_thresholds_json column repair Authored-by: Markus Hartung --- .env.example | 7 + docs/reference/ENVIRONMENT.md | 2 + .../dashboard/batch/BatchListTab.tsx | 85 +++++- .../dashboard/batch/FileDetailModal.tsx | 53 +++- .../dashboard/batch/FilesListTab.tsx | 62 ++--- src/app/(dashboard)/dashboard/batch/page.tsx | 14 +- src/app/api/v1/batches/[id]/route.ts | 42 ++- .../api/v1/batches/delete-completed/route.ts | 28 ++ src/app/api/v1/files/[id]/route.ts | 13 +- src/lib/db/batches.ts | 67 +++++ src/lib/db/core.ts | 6 + src/lib/db/migrationRunner.ts | 3 + ...er_connection_quota_window_thresholds.sql} | 0 src/lib/localDb.ts | 2 + src/server/authz/policies/clientApi.ts | 16 +- tests/unit/batch-deletion-route-logic.test.ts | 101 ++++++++ tests/unit/batch-deletion.test.ts | 242 ++++++++++++++++++ 17 files changed, 675 insertions(+), 68 deletions(-) create mode 100644 src/app/api/v1/batches/delete-completed/route.ts rename src/lib/db/migrations/{056_provider_connection_quota_window_thresholds.sql => 057_provider_connection_quota_window_thresholds.sql} (100%) create mode 100644 tests/unit/batch-deletion-route-logic.test.ts create mode 100644 tests/unit/batch-deletion.test.ts diff --git a/.env.example b/.env.example index fd1951fd5f..312f56e225 100644 --- a/.env.example +++ b/.env.example @@ -699,6 +699,13 @@ GEMINI_CLI_USER_AGENT="google-api-nodejs-client/10.3.0" # OMNIROUTE_CHATGPT_TLS_TIMEOUT_MS=60000 # OMNIROUTE_CHATGPT_TLS_GRACE_MS=10000 +# ── Claude TLS sidecar (Chromium-fingerprinted client) ── +# Used by: open-sse/services/claudeTlsClient.ts — wire-level timeout for +# the bogdanfinn/tls-client koffi binding and the JS-side grace window +# layered on top of it when the native library is wedged. +# OMNIROUTE_CLAUDE_TLS_TIMEOUT_MS=60000 +# OMNIROUTE_CLAUDE_TLS_GRACE_MS=10000 + # ── Circuit breaker thresholds and reset windows ── # Used by: open-sse/config/constants.ts → src/lib/resilience/settings.ts. # Defaults match historical PROVIDER_PROFILES values (post-scaling for diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 7724e458ed..9538a815de 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -526,6 +526,8 @@ REQUEST_TIMEOUT_MS (global override) | `OMNIROUTE_DEFAULT_FETCH_TIMEOUT_MS` | `120000` | Fallback used by `src/shared/utils/fetchTimeout.ts` when `FETCH_TIMEOUT_MS` is unset. | | `OMNIROUTE_CHATGPT_TLS_TIMEOUT_MS` | `60000` | Wire-level timeout for the bogdanfinn/tls-client koffi binding (`chatgptTlsClient.ts`). | | `OMNIROUTE_CHATGPT_TLS_GRACE_MS` | `10000` | JS-side grace added on top of the wire timeout when the native binding is wedged. | +| `OMNIROUTE_CLAUDE_TLS_TIMEOUT_MS` | `60000` | Wire-level timeout for the bogdanfinn/tls-client koffi binding (`claudeTlsClient.ts`). | +| `OMNIROUTE_CLAUDE_TLS_GRACE_MS` | `10000` | JS-side grace added on top of the wire timeout when the native binding is wedged. | ### Circuit Breaker Thresholds diff --git a/src/app/(dashboard)/dashboard/batch/BatchListTab.tsx b/src/app/(dashboard)/dashboard/batch/BatchListTab.tsx index 81323d0e5d..c47f7f21ca 100644 --- a/src/app/(dashboard)/dashboard/batch/BatchListTab.tsx +++ b/src/app/(dashboard)/dashboard/batch/BatchListTab.tsx @@ -64,6 +64,7 @@ interface BatchListTabProps { batches: BatchRecord[]; files: FileRecord[]; loading: boolean; + onRefresh?: () => void; } const STATUS_STYLES: Record = { @@ -124,10 +125,60 @@ const ALL_STATUSES = [ "expired", ]; -export default function BatchListTab({ batches, files, loading }: Readonly) { +export default function BatchListTab({ + batches, + files, + loading, + onRefresh, +}: Readonly) { const [selectedBatch, setSelectedBatch] = useState(null); const [statusFilter, setStatusFilter] = useState("all"); const [searchQuery, setSearchQuery] = useState(""); + const [removingCompleted, setRemovingCompleted] = useState(false); + const [deletingId, setDeletingId] = useState(null); + + const completedBatches = batches.filter((b) => b.status === "completed"); + + const handleDeleteBatch = async (e: React.MouseEvent, batch: BatchRecord) => { + e.stopPropagation(); + setDeletingId(batch.id); + try { + const res = await fetch(`/api/v1/batches/${batch.id}`, { method: "DELETE" }); + if (res.ok) { + onRefresh?.(); + } else { + console.error( + `[DeleteBatch] DELETE ${batch.id} returned ${res.status}`, + await res.text().catch(() => "") + ); + } + } catch (err) { + console.error(`[DeleteBatch] DELETE ${batch.id} threw`, err); + } finally { + setDeletingId(null); + } + }; + + const handleRemoveCompleted = async () => { + if (completedBatches.length === 0) return; + setRemovingCompleted(true); + try { + const res = await fetch("/api/v1/batches/delete-completed", { method: "DELETE" }); + if (res.ok) { + onRefresh?.(); + } else { + console.error( + "[RemoveCompleted] DELETE /batches/delete-completed returned", + res.status, + await res.text().catch(() => "") + ); + } + } catch (err) { + console.error("[RemoveCompleted] DELETE /batches/delete-completed threw", err); + } finally { + setRemovingCompleted(false); + } + }; const filtered = batches.filter((b) => { if (statusFilter !== "all" && b.status !== statusFilter) return false; @@ -164,6 +215,19 @@ export default function BatchListTab({ batches, files, loading }: Readonly ))} + {/* Table */} @@ -192,12 +256,13 @@ export default function BatchListTab({ batches, files, loading }: Readonly Expires + {loading && filtered.length === 0 ? ( - +
Loading… @@ -206,7 +271,7 @@ export default function BatchListTab({ batches, files, loading }: Readonly ) : filtered.length === 0 ? ( - + No batches found @@ -269,6 +334,20 @@ export default function BatchListTab({ batches, files, loading }: Readonly {batch.expiresAt ? relativeTime(batch.expiresAt) : "—"} + + {["completed", "failed", "cancelled", "expired"].includes(batch.status) && ( + + )} + ); }) diff --git a/src/app/(dashboard)/dashboard/batch/FileDetailModal.tsx b/src/app/(dashboard)/dashboard/batch/FileDetailModal.tsx index 46add9c758..4281178a7d 100644 --- a/src/app/(dashboard)/dashboard/batch/FileDetailModal.tsx +++ b/src/app/(dashboard)/dashboard/batch/FileDetailModal.tsx @@ -44,10 +44,21 @@ interface FileRecord { expiresAt?: number | null; } +interface BatchRecord { + id: string; + endpoint: string; + status: string; + inputFileId: string; + outputFileId?: string | null; + errorFileId?: string | null; + model?: string | null; +} + interface FileDetailModalProps { file: FileRecord; contents: string | null; loading: boolean; + batches?: BatchRecord[]; onClose: () => void; } @@ -55,10 +66,15 @@ export default function FileDetailModal({ file, contents, loading, + batches, onClose, }: Readonly) { const [copied, setCopied] = useState(false); + const relatedBatches = (batches ?? []).filter( + (b) => b.inputFileId === file.id || b.outputFileId === file.id || b.errorFileId === file.id + ); + useEffect(() => { const handler = (e: KeyboardEvent) => { if (e.key === "Escape") onClose(); @@ -84,8 +100,8 @@ export default function FileDetailModal({ } }; - const createdAtTs = file.created_at ?? file.createdAt; - const expiresAtTs = file.expires_at ?? file.expiresAt; + const createdAtTs = file.createdAt; + const expiresAtTs = file.expiresAt; const lineCount = contents ? contents.split("\n").filter((l) => l.trim()).length : 0; const isTruncated = lineCount > 1000; @@ -176,6 +192,39 @@ export default function FileDetailModal({
+ {/* Related batches */} + {relatedBatches.length > 0 && ( +
+

+ Used by {relatedBatches.length} batch{relatedBatches.length > 1 ? "es" : ""} +

+
+ {relatedBatches.map((b) => ( +
+ + pending_actions + + {b.id} + + {b.status.replaceAll("_", " ")} + +
+ ))} +
+
+ )} + {/* Contents */}
diff --git a/src/app/(dashboard)/dashboard/batch/FilesListTab.tsx b/src/app/(dashboard)/dashboard/batch/FilesListTab.tsx index 0cb20d2728..ea0550b72e 100644 --- a/src/app/(dashboard)/dashboard/batch/FilesListTab.tsx +++ b/src/app/(dashboard)/dashboard/batch/FilesListTab.tsx @@ -39,10 +39,21 @@ interface FileRecord { expiresAt?: number | null; } +interface BatchRecord { + id: string; + endpoint: string; + status: string; + inputFileId: string; + outputFileId?: string | null; + errorFileId?: string | null; + model?: string | null; +} + interface FilesListTabProps { files: FileRecord[]; loading: boolean; onRefresh?: () => void; + batches?: BatchRecord[]; } const PURPOSE_STYLES_MAP: Record = { @@ -67,13 +78,17 @@ function formatBytes(bytes: number): string { return `${(bytes / 1024 / 1024).toFixed(2)} MB`; } -export default function FilesListTab({ files, loading, onRefresh }: Readonly) { +export default function FilesListTab({ + files, + loading, + onRefresh, + batches, +}: Readonly) { const [searchQuery, setSearchQuery] = useState(""); const [purposeFilter, setPurposeFilter] = useState("all"); const [selectedFileId, setSelectedFileId] = useState(null); const [fileContents, setFileContents] = useState(null); const [contentsLoading, setContentsLoading] = useState(false); - const [deleteInProgress, setDeleteInProgress] = useState(null); const purposes = ["all", ...Array.from(new Set(files.map((f) => f.purpose)))]; @@ -108,31 +123,6 @@ export default function FilesListTab({ files, loading, onRefresh }: Readonly { - e.stopPropagation(); - if (!confirm("Are you sure you want to delete this file?")) return; - - setDeleteInProgress(fileId); - try { - const response = await fetch(`/api/v1/files/${fileId}`, { method: "DELETE" }); - if (response.ok) { - // File deleted successfully - refresh list - if (selectedFileId === fileId) { - setSelectedFileId(null); - setFileContents(null); - } - if (onRefresh) onRefresh(); - } else { - alert("Failed to delete file"); - } - } catch (error) { - console.error("Failed to delete file:", error); - alert("Error deleting file"); - } finally { - setDeleteInProgress(null); - } - }; - return (
{/* Filters */} @@ -180,13 +170,12 @@ export default function FilesListTab({ files, loading, onRefresh }: Readonly Expires - {loading && filtered.length === 0 ? ( - +
Loading… @@ -195,7 +184,7 @@ export default function FilesListTab({ files, loading, onRefresh }: Readonly ) : filtered.length === 0 ? ( - + No files found @@ -235,18 +224,6 @@ export default function FilesListTab({ files, loading, onRefresh }: Readonly {fileExpiresAt ? relativeExpiration(fileExpiresAt) : "Never"} - - - ); }) @@ -261,6 +238,7 @@ export default function FilesListTab({ files, loading, onRefresh }: Readonly setSelectedFileId(null)} /> )} diff --git a/src/app/(dashboard)/dashboard/batch/page.tsx b/src/app/(dashboard)/dashboard/batch/page.tsx index a7c6e3cd13..6afc254877 100644 --- a/src/app/(dashboard)/dashboard/batch/page.tsx +++ b/src/app/(dashboard)/dashboard/batch/page.tsx @@ -206,7 +206,12 @@ export default function BatchPage() {
{activeTab === "batches" ? ( <> - + fetchData(false)} + /> {loadingMore && batchesCount > 0 && (
Loading more…
)} @@ -214,7 +219,12 @@ export default function BatchPage() { ) : ( <> - fetchData(false)} /> + fetchData(false)} + batches={batches} + /> {loadingMore && filesCount > 0 && (
Loading more…
)} diff --git a/src/app/api/v1/batches/[id]/route.ts b/src/app/api/v1/batches/[id]/route.ts index e85aa336c4..7cca8d8cfc 100644 --- a/src/app/api/v1/batches/[id]/route.ts +++ b/src/app/api/v1/batches/[id]/route.ts @@ -1,5 +1,5 @@ import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors"; -import { getBatch } from "@/lib/localDb"; +import { getBatch, deleteBatch } from "@/lib/localDb"; import { NextResponse } from "next/server"; import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope"; @@ -38,15 +38,23 @@ export async function OPTIONS() { return handleCorsOptions(); } +function scopeCheck( + scope: { isSessionAuth: boolean; apiKeyId: string | null }, + recordApiKeyId: string | null | undefined +): boolean { + if (scope.isSessionAuth) return true; + if (recordApiKeyId === null || recordApiKeyId === undefined) return true; + return recordApiKeyId === scope.apiKeyId; +} + export async function GET(request: Request, { params }: { params: Promise<{ id: string }> }) { const scope = await getApiKeyRequestScope(request); if (scope.rejection) return scope.rejection; - const apiKeyId = scope.apiKeyId; const { id } = await params; const batch = getBatch(id); - if (!batch || (batch.apiKeyId !== null && batch.apiKeyId !== apiKeyId)) { + if (!batch || !scopeCheck(scope, batch.apiKeyId)) { return NextResponse.json( { error: { message: "Batch not found", type: "invalid_request_error" } }, { status: 404, headers: CORS_HEADERS } @@ -55,3 +63,31 @@ export async function GET(request: Request, { params }: { params: Promise<{ id: return NextResponse.json(formatBatchResponse(batch), { headers: CORS_HEADERS }); } + +export async function DELETE(request: Request, { params }: { params: Promise<{ id: string }> }) { + const scope = await getApiKeyRequestScope(request); + if (scope.rejection) return scope.rejection; + + const { id } = await params; + const batch = getBatch(id); + + if (!batch || !scopeCheck(scope, batch.apiKeyId)) { + return NextResponse.json( + { error: { message: "Batch not found", type: "invalid_request_error" } }, + { status: 404, headers: CORS_HEADERS } + ); + } + + // Only allow deleting terminal batches (completed, failed, cancelled, expired) + const terminal = ["completed", "failed", "cancelled", "expired"]; + if (!terminal.includes(batch.status)) { + return NextResponse.json( + { error: { message: "Only terminal batches can be deleted", type: "invalid_request_error" } }, + { status: 409, headers: CORS_HEADERS } + ); + } + + deleteBatch(id); + + return NextResponse.json({ id, object: "batch", deleted: true }, { headers: CORS_HEADERS }); +} diff --git a/src/app/api/v1/batches/delete-completed/route.ts b/src/app/api/v1/batches/delete-completed/route.ts new file mode 100644 index 0000000000..794f96cad4 --- /dev/null +++ b/src/app/api/v1/batches/delete-completed/route.ts @@ -0,0 +1,28 @@ +import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors"; +import { deleteCompletedBatches } from "@/lib/localDb"; +import { NextResponse } from "next/server"; +import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope"; + +export async function OPTIONS() { + return handleCorsOptions(); +} + +export async function DELETE(request: Request) { + const scope = await getApiKeyRequestScope(request); + if (scope.rejection) return scope.rejection; + + // Allow session-authenticated (dashboard) requests; for API-key requests, require a key + if (!scope.isSessionAuth && !scope.apiKeyId) { + return NextResponse.json( + { error: { message: "Authentication required", type: "invalid_request_error" } }, + { status: 401, headers: CORS_HEADERS } + ); + } + + const result = deleteCompletedBatches(); + + return NextResponse.json( + { deleted: true, deletedBatches: result.deletedBatches, deletedFiles: result.deletedFiles }, + { headers: CORS_HEADERS } + ); +} diff --git a/src/app/api/v1/files/[id]/route.ts b/src/app/api/v1/files/[id]/route.ts index 5457349553..66565e52ec 100644 --- a/src/app/api/v1/files/[id]/route.ts +++ b/src/app/api/v1/files/[id]/route.ts @@ -15,7 +15,7 @@ export async function GET(request: Request, { params }: { params: Promise<{ id: const { id } = await params; const file = getFile(id); - if (!file || (file.apiKeyId !== null && file.apiKeyId !== apiKeyId)) { + if (!file || (file.apiKeyId !== null && file.apiKeyId !== apiKeyId && !scope.isSessionAuth)) { return NextResponse.json( { error: { message: "File not found", type: "invalid_request_error" } }, { status: 404, headers: CORS_HEADERS } @@ -33,7 +33,16 @@ export async function DELETE(request: Request, { params }: { params: Promise<{ i const { id } = await params; const file = getFile(id); - if (!file || (file.apiKeyId !== null && file.apiKeyId !== apiKeyId)) { + if (!file) { + return NextResponse.json( + { error: { message: "File not found", type: "invalid_request_error" } }, + { status: 404, headers: CORS_HEADERS } + ); + } + + // Allow session-authenticated (dashboard) requests to delete any file; + // for API-key-authenticated requests, enforce scope. + if (!scope.isSessionAuth && file.apiKeyId !== null && file.apiKeyId !== apiKeyId) { return NextResponse.json( { error: { message: "File not found", type: "invalid_request_error" } }, { status: 404, headers: CORS_HEADERS } diff --git a/src/lib/db/batches.ts b/src/lib/db/batches.ts index c88919985e..efb009ea4d 100644 --- a/src/lib/db/batches.ts +++ b/src/lib/db/batches.ts @@ -1,4 +1,5 @@ import { getDbInstance, rowToCamel, objToSnake } from "./core"; +import { deleteFile } from "./files"; import { v4 as uuidv4 } from "uuid"; function parseBatchRow(row: any): BatchRecord { @@ -212,3 +213,69 @@ export function getTerminalBatches(): BatchRecord[] { .all(); return rows.map((row) => parseBatchRow(row)); } + +export function deleteBatch(id: string): boolean { + const db = getDbInstance(); + const batch = getBatch(id); + if (!batch) return false; + + // Soft-delete associated files (input, output, error) + if (batch.inputFileId) { + try { + deleteFile(batch.inputFileId); + } catch { + /* ignore */ + } + } + if (batch.outputFileId) { + try { + deleteFile(batch.outputFileId); + } catch { + /* ignore */ + } + } + if (batch.errorFileId) { + try { + deleteFile(batch.errorFileId); + } catch { + /* ignore */ + } + } + + const result = db.prepare("DELETE FROM batches WHERE id = ?").run(id); + return result.changes > 0; +} + +export function deleteCompletedBatches(): { deletedBatches: number; deletedFiles: number } { + const db = getDbInstance(); + + // Collect unique file IDs from all completed batches + const rows = db + .prepare( + "SELECT input_file_id, output_file_id, error_file_id FROM batches WHERE status = 'completed'" + ) + .all() as Array<{ + input_file_id: string | null; + output_file_id: string | null; + error_file_id: string | null; + }>; + + const fileIds = new Set(); + for (const row of rows) { + if (row.input_file_id) fileIds.add(row.input_file_id); + if (row.output_file_id) fileIds.add(row.output_file_id); + if (row.error_file_id) fileIds.add(row.error_file_id); + } + + let deletedFiles = 0; + for (const fid of fileIds) { + try { + if (deleteFile(fid)) deletedFiles++; + } catch { + /* ignore */ + } + } + + const result = db.prepare("DELETE FROM batches WHERE status = 'completed'").run(); + return { deletedBatches: result.changes, deletedFiles }; +} diff --git a/src/lib/db/core.ts b/src/lib/db/core.ts index 59bb37dc92..f2ffcf8f9a 100644 --- a/src/lib/db/core.ts +++ b/src/lib/db/core.ts @@ -188,6 +188,7 @@ const SCHEMA_SQL = ` last_used_at TEXT, "group" TEXT, max_concurrent INTEGER, + quota_window_thresholds_json TEXT, created_at TEXT NOT NULL, updated_at TEXT NOT NULL ); @@ -520,6 +521,10 @@ function ensureProviderConnectionsColumns(db: SqliteDatabase) { db.exec("ALTER TABLE provider_connections ADD COLUMN max_concurrent INTEGER"); console.log("[DB] Added provider_connections.max_concurrent column"); } + if (!columnNames.has("quota_window_thresholds_json")) { + db.exec("ALTER TABLE provider_connections ADD COLUMN quota_window_thresholds_json TEXT"); + console.log("[DB] Added provider_connections.quota_window_thresholds_json column"); + } db.exec( "CREATE INDEX IF NOT EXISTS idx_pc_max_concurrent ON provider_connections(provider, max_concurrent)" ); @@ -1132,6 +1137,7 @@ export function getDbInstance(): SqliteDatabase { memoryDb.exec(SCHEMA_SQL); ensureUsageHistoryColumns(memoryDb); ensureCallLogsColumns(memoryDb); + ensureProviderConnectionsColumns(memoryDb); setDb(memoryDb); return memoryDb; } diff --git a/src/lib/db/migrationRunner.ts b/src/lib/db/migrationRunner.ts index d53c66e9a9..578e9e5acf 100644 --- a/src/lib/db/migrationRunner.ts +++ b/src/lib/db/migrationRunner.ts @@ -148,6 +148,9 @@ const LEGACY_VERSION_SLOT_MIGRATIONS = [ { version: "031", name: "api_keys_expires" }, { version: "032", name: "detailed_logs_warnings" }, { version: "033", name: "provider_connections_block_extra_usage" }, + { version: "033", name: "add_batch_id_to_call_logs" }, + { version: "046", name: "remove_status_from_files" }, + { version: "051", name: "remove_status_from_files" }, ] as const; const SUPERSEDED_DUPLICATE_MIGRATIONS = [ diff --git a/src/lib/db/migrations/056_provider_connection_quota_window_thresholds.sql b/src/lib/db/migrations/057_provider_connection_quota_window_thresholds.sql similarity index 100% rename from src/lib/db/migrations/056_provider_connection_quota_window_thresholds.sql rename to src/lib/db/migrations/057_provider_connection_quota_window_thresholds.sql diff --git a/src/lib/localDb.ts b/src/lib/localDb.ts index 65a5681e08..523d9b8ff0 100755 --- a/src/lib/localDb.ts +++ b/src/lib/localDb.ts @@ -261,6 +261,8 @@ export { countBatches, getPendingBatches, getTerminalBatches, + deleteBatch, + deleteCompletedBatches, } from "./db/batches"; export type { FileRecord } from "./db/files"; diff --git a/src/server/authz/policies/clientApi.ts b/src/server/authz/policies/clientApi.ts index 47022b1919..f56afe0b4b 100644 --- a/src/server/authz/policies/clientApi.ts +++ b/src/server/authz/policies/clientApi.ts @@ -1,4 +1,4 @@ -import { isDashboardSessionAuthenticated } from "../../../shared/utils/apiAuth"; +import { isDashboardSessionAuthenticated } from "@/shared/utils/apiAuth.ts"; import type { AuthOutcome, PolicyContext, RoutePolicy } from "../context"; import { allow, reject } from "../context"; @@ -15,24 +15,12 @@ function maskKeyId(apiKey: string): string { return `key_${tail}`; } -function isDashboardModelCatalogRead(ctx: PolicyContext): boolean { - const method = ctx.request.method.toUpperCase(); - if (method !== "GET" && method !== "HEAD") return false; - return ( - ctx.classification.normalizedPath === "/api/v1/models" || - ctx.classification.normalizedPath === "/api/v1" - ); -} - export const clientApiPolicy: RoutePolicy = { routeClass: "CLIENT_API", async evaluate(ctx: PolicyContext): Promise { const bearer = extractBearer(ctx.request.headers); if (!bearer) { - if ( - isDashboardModelCatalogRead(ctx) && - (await isDashboardSessionAuthenticated(ctx.request)) - ) { + if (await isDashboardSessionAuthenticated(ctx.request)) { return allow({ kind: "dashboard_session", id: "dashboard" }); } diff --git a/tests/unit/batch-deletion-route-logic.test.ts b/tests/unit/batch-deletion-route-logic.test.ts new file mode 100644 index 0000000000..a5066325f9 --- /dev/null +++ b/tests/unit/batch-deletion-route-logic.test.ts @@ -0,0 +1,101 @@ +import { test } from "node:test"; +import assert from "node:assert"; + +// Tests for the business logic embedded in DELETE route handlers. +// These verify every code path without importing Next.js route modules +// (which pull in pino/thread-stream — broken on Node 26). + +const TERMINAL = ["completed", "failed", "cancelled", "expired"]; + +function scopeCheck( + isSessionAuth: boolean, + recordApiKeyId: string | null | undefined, + apiKeyId: string | null +): boolean { + if (isSessionAuth) return true; + if (recordApiKeyId === null || recordApiKeyId === undefined) return true; + return recordApiKeyId === apiKeyId; +} + +function canDeleteBatch(status: string): boolean { + return TERMINAL.includes(status); +} + +test("scopeCheck — session auth always passes", () => { + assert.strictEqual(scopeCheck(true, "key-1", "key-1"), true); + assert.strictEqual(scopeCheck(true, "key-1", "different-key"), true); + assert.strictEqual(scopeCheck(true, null, null), true); + assert.strictEqual(scopeCheck(true, undefined, null), true); +}); + +test("scopeCheck — null record ApiKeyId always passes", () => { + assert.strictEqual(scopeCheck(false, null, null), true); + assert.strictEqual(scopeCheck(false, null, "any-key"), true); + assert.strictEqual(scopeCheck(false, undefined, null), true); + assert.strictEqual(scopeCheck(false, undefined, "any-key"), true); +}); + +test("scopeCheck — matching apiKeyId passes", () => { + assert.strictEqual(scopeCheck(false, "key-1", "key-1"), true); +}); + +test("scopeCheck — mismatched apiKeyId fails", () => { + assert.strictEqual(scopeCheck(false, "key-1", null), false); + assert.strictEqual(scopeCheck(false, "key-1", "key-2"), false); +}); + +test("batch deletion only allowed for terminal statuses", () => { + for (const s of ["completed", "failed", "cancelled", "expired"]) { + assert.strictEqual(canDeleteBatch(s), true, `${s} should be deletable`); + } + for (const s of ["validating", "in_progress", "finalizing", "cancelling"]) { + assert.strictEqual(canDeleteBatch(s), false, `${s} should NOT be deletable`); + } +}); + +test("delete completed auth — requires session or API key", () => { + // Simulates the check in delete-completed/route.ts: + // if (!scope.isSessionAuth && !scope.apiKeyId) → 401 + function needsAuth(isSessionAuth: boolean, apiKeyId: string | null): boolean { + return !isSessionAuth && !apiKeyId; + } + assert.strictEqual(needsAuth(true, null), false, "session auth → OK"); + assert.strictEqual(needsAuth(true, "key-1"), false, "session auth + key → OK"); + assert.strictEqual(needsAuth(false, "key-1"), false, "API key → OK"); + assert.strictEqual(needsAuth(false, null), true, "no auth → 401"); +}); + +test("response JSON shape for single batch deletion", () => { + const id = "batch_test123"; + const body = { id, object: "batch", deleted: true }; + assert.strictEqual(body.id, id); + assert.strictEqual(body.object, "batch"); + assert.strictEqual(body.deleted, true); +}); + +test("response JSON shape for delete-completed", () => { + const body = { deleted: true, deletedBatches: 3, deletedFiles: 5 }; + assert.strictEqual(body.deleted, true); + assert.strictEqual(body.deletedBatches, 3); + assert.strictEqual(body.deletedFiles, 5); +}); + +test("response JSON shape for 404 error", () => { + const body = { error: { message: "Batch not found", type: "invalid_request_error" } }; + assert.strictEqual(body.error.message, "Batch not found"); + assert.strictEqual(body.error.type, "invalid_request_error"); +}); + +test("response JSON shape for 409 error", () => { + const body = { + error: { message: "Only terminal batches can be deleted", type: "invalid_request_error" }, + }; + assert.strictEqual(body.error.message, "Only terminal batches can be deleted"); + assert.strictEqual(body.error.type, "invalid_request_error"); +}); + +test("response JSON shape for 401 error", () => { + const body = { error: { message: "Authentication required", type: "invalid_request_error" } }; + assert.strictEqual(body.error.message, "Authentication required"); + assert.strictEqual(body.error.type, "invalid_request_error"); +}); diff --git a/tests/unit/batch-deletion.test.ts b/tests/unit/batch-deletion.test.ts new file mode 100644 index 0000000000..4a0043e721 --- /dev/null +++ b/tests/unit/batch-deletion.test.ts @@ -0,0 +1,242 @@ +import { describe, it } from "node:test"; +import assert from "node:assert"; +import { + createFile, + createBatch, + getBatch, + deleteBatch, + deleteCompletedBatches, + getFile, + deleteFile, +} from "@/lib/localDb"; + +describe("deleteBatch", () => { + it("should delete a single batch and its associated files", () => { + const inputFile = createFile({ + bytes: 10, + filename: "single-delete-input.jsonl", + purpose: "batch", + content: Buffer.from("{}"), + }); + + const batch = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: inputFile.id, + status: "completed", + }); + + assert.ok(getBatch(batch.id)); + assert.ok(getFile(inputFile.id)); + + const result = deleteBatch(batch.id); + assert.strictEqual(result, true); + + assert.strictEqual(getBatch(batch.id), null); + assert.strictEqual(getFile(inputFile.id), null); + }); + + it("should return false for a non-existent batch id", () => { + const result = deleteBatch("batch_nonexistent"); + assert.strictEqual(result, false); + }); + + it("should delete a batch with all three file references", () => { + const inputFile = createFile({ + bytes: 10, + filename: "delete-all-input.jsonl", + purpose: "batch", + content: Buffer.from("input"), + }); + const outputFile = createFile({ + bytes: 20, + filename: "delete-all-output.jsonl", + purpose: "batch", + content: Buffer.from("output"), + }); + const errorFile = createFile({ + bytes: 30, + filename: "delete-all-error.jsonl", + purpose: "batch", + content: Buffer.from("error"), + }); + + const batch = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: inputFile.id, + outputFileId: outputFile.id, + errorFileId: errorFile.id, + status: "completed", + }); + + assert.ok(getFile(inputFile.id)); + assert.ok(getFile(outputFile.id)); + assert.ok(getFile(errorFile.id)); + + const result = deleteBatch(batch.id); + assert.strictEqual(result, true); + + assert.strictEqual(getBatch(batch.id), null); + assert.strictEqual(getFile(inputFile.id), null); + assert.strictEqual(getFile(outputFile.id), null); + assert.strictEqual(getFile(errorFile.id), null); + }); + + it("should delete a batch whose files were already deleted", () => { + const f = createFile({ + bytes: 10, + filename: "already-deleted-input.jsonl", + purpose: "batch", + content: Buffer.from("x"), + }); + const batch = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: f.id, + status: "completed", + }); + + // Delete the file first + deleteFile(f.id); + + assert.strictEqual(getFile(f.id), null); + assert.ok(getBatch(batch.id)); + + const result = deleteBatch(batch.id); + assert.strictEqual(result, true); + assert.strictEqual(getBatch(batch.id), null); + }); + + it("should delete a batch regardless of status", () => { + for (const status of [ + "validating", + "in_progress", + "finalizing", + "cancelling", + "failed", + "cancelled", + "expired", + ] as const) { + const f = createFile({ + bytes: 10, + filename: `delete-status-${status}.jsonl`, + purpose: "batch", + content: Buffer.from("x"), + }); + const b = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: f.id, + status, + }); + assert.ok(getBatch(b.id), `batch with status '${status}' should exist`); + assert.strictEqual( + deleteBatch(b.id), + true, + `deleteBatch for status '${status}' should succeed` + ); + assert.strictEqual(getBatch(b.id), null, `batch with status '${status}' should be gone`); + assert.strictEqual(getFile(f.id), null, `file for status '${status}' should be gone`); + } + }); +}); + +describe("deleteCompletedBatches", () => { + it("should delete all completed batches and their associated files", () => { + // Create 3 completed batches with their own files + const batchIds: string[] = []; + const fileIds: string[] = []; + + for (let i = 0; i < 3; i++) { + const inputFile = createFile({ + bytes: 10, + filename: `bulk-input-${i}.jsonl`, + purpose: "batch", + content: Buffer.from("{}"), + }); + fileIds.push(inputFile.id); + + const batch = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: inputFile.id, + status: "completed", + }); + batchIds.push(batch.id); + } + + // Create a non-completed batch that should survive + const liveInput = createFile({ + bytes: 10, + filename: "live-input.jsonl", + purpose: "batch", + content: Buffer.from("{}"), + }); + const liveBatch = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: liveInput.id, + status: "in_progress", + }); + + // Verify everything exists + for (const id of batchIds) assert.ok(getBatch(id), `batch ${id} should exist`); + for (const id of fileIds) assert.ok(getFile(id), `file ${id} should exist`); + assert.ok(getBatch(liveBatch.id)); + assert.ok(getFile(liveInput.id)); + + // Delete all completed (may include pre-existing ones from other tests) + const result = deleteCompletedBatches(); + assert.ok(result.deletedBatches >= 3, `expected >=3, got ${result.deletedBatches}`); + assert.ok(result.deletedFiles >= 3, `expected >=3, got ${result.deletedFiles}`); + + // Verify completed batches and their files are gone + for (const id of batchIds) assert.strictEqual(getBatch(id), null); + for (const id of fileIds) assert.strictEqual(getFile(id), null); + + // Verify non-completed batch and its file survive + assert.ok(getBatch(liveBatch.id), "non-completed batch should survive"); + assert.ok(getFile(liveInput.id), "non-completed batch's file should survive"); + }); + + it("should return zero counts when no completed batches exist", () => { + const result = deleteCompletedBatches(); + assert.strictEqual(result.deletedBatches, 0); + assert.strictEqual(result.deletedFiles, 0); + }); + + it("should handle shared file IDs across multiple completed batches", () => { + const sharedFile = createFile({ + bytes: 10, + filename: "shared-input.jsonl", + purpose: "batch", + content: Buffer.from("shared"), + }); + + const batchA = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: sharedFile.id, + status: "completed", + }); + const batchB = createBatch({ + endpoint: "/v1/chat/completions", + completionWindow: "24h", + inputFileId: sharedFile.id, + status: "completed", + }); + + assert.ok(getBatch(batchA.id)); + assert.ok(getBatch(batchB.id)); + assert.ok(getFile(sharedFile.id)); + + const result = deleteCompletedBatches(); + assert.ok(result.deletedBatches >= 2); + assert.ok(result.deletedFiles >= 1, "shared file should be counted once"); + + assert.strictEqual(getBatch(batchA.id), null); + assert.strictEqual(getBatch(batchB.id), null); + assert.strictEqual(getFile(sharedFile.id), null); + }); +});