fix: close MCP audit SQLite connections on shutdown (#1348)
Integrated into release/v3.6.7
This commit is contained in:
@@ -20,6 +20,7 @@
|
||||
|
||||
x-common: &common
|
||||
restart: unless-stopped
|
||||
stop_grace_period: 40s
|
||||
env_file: .env
|
||||
environment:
|
||||
- DATA_DIR=/app/data # Must match the volume mount below
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
type MockAuditDb = {
|
||||
prepare: ReturnType<typeof vi.fn>;
|
||||
pragma: ReturnType<typeof vi.fn>;
|
||||
close: ReturnType<typeof vi.fn>;
|
||||
open?: boolean;
|
||||
};
|
||||
|
||||
function createStatementMock() {
|
||||
return {
|
||||
get: vi.fn(),
|
||||
all: vi.fn(),
|
||||
run: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describe("MCP audit shutdown", () => {
|
||||
let dataDir: string;
|
||||
let dbFile: string;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-mcp-audit-"));
|
||||
dbFile = path.join(dataDir, "storage.sqlite");
|
||||
fs.writeFileSync(dbFile, "");
|
||||
process.env.DATA_DIR = dataDir;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete process.env.DATA_DIR;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("checkpoints and closes the audit database during shutdown", async () => {
|
||||
const mockDb: MockAuditDb = {
|
||||
prepare: vi.fn(() => createStatementMock()),
|
||||
pragma: vi.fn(),
|
||||
close: vi.fn(),
|
||||
open: true,
|
||||
};
|
||||
const MockDatabase = vi.fn(function MockDatabase() {
|
||||
return mockDb;
|
||||
});
|
||||
|
||||
vi.doMock("better-sqlite3", () => ({
|
||||
default: MockDatabase,
|
||||
}));
|
||||
|
||||
const audit = await import("../audit.ts");
|
||||
|
||||
await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 12, true);
|
||||
expect(mockDb.prepare).toHaveBeenCalledTimes(1);
|
||||
|
||||
expect(audit.closeAuditDb()).toBe(true);
|
||||
expect(mockDb.pragma).toHaveBeenCalledWith("wal_checkpoint(TRUNCATE)");
|
||||
expect(mockDb.close).toHaveBeenCalledTimes(1);
|
||||
expect(audit.closeAuditDb()).toBe(false);
|
||||
});
|
||||
|
||||
it("still closes the audit database when checkpoint fails", async () => {
|
||||
const mockDb: MockAuditDb = {
|
||||
prepare: vi.fn(() => createStatementMock()),
|
||||
pragma: vi.fn(() => {
|
||||
throw new Error("database is busy");
|
||||
}),
|
||||
close: vi.fn(),
|
||||
open: true,
|
||||
};
|
||||
const MockDatabase = vi.fn(function MockDatabase() {
|
||||
return mockDb;
|
||||
});
|
||||
|
||||
vi.doMock("better-sqlite3", () => ({
|
||||
default: MockDatabase,
|
||||
}));
|
||||
|
||||
const audit = await import("../audit.ts");
|
||||
|
||||
await audit.logToolCall("omniroute_get_health", {}, {}, 5, true);
|
||||
expect(audit.closeAuditDb()).toBe(true);
|
||||
expect(mockDb.close).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -18,6 +18,9 @@ interface StatementLike<TRow = unknown> {
|
||||
|
||||
interface AuditDatabase {
|
||||
prepare: <TRow = unknown>(sql: string) => StatementLike<TRow>;
|
||||
pragma: (sql: string) => unknown;
|
||||
close: () => void;
|
||||
open?: boolean;
|
||||
}
|
||||
|
||||
interface AuditStatsRow {
|
||||
@@ -171,6 +174,33 @@ async function getDb(): Promise<AuditDatabase | null> {
|
||||
}
|
||||
}
|
||||
|
||||
export function closeAuditDb(): boolean {
|
||||
if (!db) return false;
|
||||
|
||||
const database = db;
|
||||
db = null;
|
||||
|
||||
try {
|
||||
try {
|
||||
if (database.open !== false) {
|
||||
database.pragma("wal_checkpoint(TRUNCATE)");
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
console.warn("[MCP Audit] WAL checkpoint failed during close:", message);
|
||||
}
|
||||
} finally {
|
||||
try {
|
||||
database.close();
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
console.warn("[MCP Audit] Failed to close database:", message);
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
// ============ Audit Logger ============
|
||||
|
||||
/**
|
||||
|
||||
@@ -43,7 +43,7 @@ import {
|
||||
} from "./schemas/tools.ts";
|
||||
import { startMcpHeartbeat } from "./runtimeHeartbeat.ts";
|
||||
|
||||
import { logToolCall } from "./audit.ts";
|
||||
import { closeAuditDb, logToolCall } from "./audit.ts";
|
||||
import {
|
||||
evaluateToolScopes,
|
||||
resolveCallerScopeContext,
|
||||
@@ -876,6 +876,9 @@ export async function startMcpStdio(): Promise<void> {
|
||||
await server.connect(transport);
|
||||
console.error("[MCP] OmniRoute MCP Server connected and ready.");
|
||||
} finally {
|
||||
if (closeAuditDb()) {
|
||||
console.error("[MCP] Audit database checkpointed and closed.");
|
||||
}
|
||||
stopHeartbeatOnce();
|
||||
process.off("exit", stopHeartbeatOnce);
|
||||
process.off("SIGINT", stopHeartbeatOnce);
|
||||
|
||||
@@ -96,7 +96,13 @@ async function waitForDrain(): Promise<void> {
|
||||
*/
|
||||
async function cleanup(): Promise<void> {
|
||||
try {
|
||||
const { closeDbInstance } = await import("@/lib/db/core");
|
||||
const [{ closeAuditDb }, { closeDbInstance }] = await Promise.all([
|
||||
import("@omniroute/open-sse/mcp-server/audit.ts"),
|
||||
import("@/lib/db/core"),
|
||||
]);
|
||||
if (closeAuditDb()) {
|
||||
console.log("[Shutdown] MCP audit database checkpointed and closed.");
|
||||
}
|
||||
if (closeDbInstance()) {
|
||||
console.log("[Shutdown] SQLite database checkpointed and closed.");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user