Repository navigation
fix: close MCP audit SQLite connections on shutdown - #1348
Merged
diegosouzapw merged 1 commit intoApr 16, 2026
Merged
diegosouzapw merged 1 commit into
diegosouzapw merged 1 commit into
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses SQLite WAL sidecar files persisting after shutdown by ensuring the MCP audit logger’s separate better-sqlite3 connection is checkpointed and closed during teardown, and by aligning Docker Compose stop timing with the app’s graceful shutdown window.
Changes:
- Add
closeAuditDb()to checkpoint/close the MCP audit SQLite connection during shutdown and stdio server teardown. - Add Vitest regression tests covering audit DB checkpoint/close behavior (including checkpoint failure).
- Increase Docker Compose
stop_grace_periodto 40s to accommodate the shutdown timeout and cleanup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/gracefulShutdown.ts | Invokes MCP audit DB shutdown alongside the main DB shutdown during graceful cleanup. |
| open-sse/mcp-server/server.ts | Ensures audit DB is closed when the stdio MCP server exits. |
| open-sse/mcp-server/audit.ts | Adds explicit WAL checkpoint + close logic for the audit DB connection. |
| open-sse/mcp-server/tests/audit.test.ts | Adds regression coverage for audit DB shutdown behavior via mocked better-sqlite3. |
| docker-compose.yml | Extends stop grace period to allow cleanup to complete in container stops. |
Comment on lines
+177
to
+202
| 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; | ||
| } |
Comment on lines
147
to
+188
| async function getDb(): Promise<AuditDatabase | null> { | ||
| if (db) return db; | ||
|
|
||
| try { | ||
| // Try importing the db module from the main app | ||
| const { homedir } = await import("node:os"); | ||
| const { join } = await import("node:path"); | ||
| const { existsSync } = await import("node:fs"); | ||
|
|
||
| const dbPath = process.env.DATA_DIR | ||
| ? join(process.env.DATA_DIR, "storage.sqlite") | ||
| : join(homedir(), ".omniroute", "storage.sqlite"); | ||
|
|
||
| if (!existsSync(dbPath)) { | ||
| console.error(`[MCP Audit] Database not found at ${dbPath} — audit logging disabled`); | ||
| return null; | ||
| } | ||
|
|
||
| const Database = (await import("better-sqlite3")).default as unknown as new ( | ||
| dbPath: string | ||
| ) => AuditDatabase; | ||
| db = new Database(dbPath); | ||
| return db; | ||
| } catch (err: unknown) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| console.error("[MCP Audit] Failed to connect to database:", message); | ||
| return 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) { |
Comment on lines
+23
to
+34
| 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; |
Comment on lines
+99
to
+105
| 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."); | ||
| } |
Owner
|
Thanks @rdself for this great contribution! 🎉 This PR has been evaluated and successfully integrated into the release/v3.6.7 branch and will be part of the final production release. We appreciate your effort! |
This was referenced Apr 17, 2026
Merged
Poid-ZA
pushed a commit
to Poid-ZA/OmniRoute
that referenced
this pull request
Aug 5, 2026
Integrated into release/v3.6.7
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
Integrated into release/v3.6.7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
storage.sqliteRoot Cause
OmniRoute already checkpoints and closes the main SQLite singleton during graceful shutdown, but the MCP audit logger kept a separate
better-sqlite3connection open without a matching shutdown path. That extra connection could keepstorage.sqlite-walandstorage.sqlite-shmon disk after an otherwise normal stop, especially in Docker when the container stop timeout was shorter than the app shutdown timeout.Impact
docker stopanddocker compose downhave enough time to finish WAL cleanup with the bundled Compose setupValidation
./node_modules/.bin/vitest run --config vitest.mcp.config.ts open-sse/mcp-server/__tests__/audit.test.ts open-sse/mcp-server/__tests__/essentialTools.test.ts./node_modules/.bin/eslint open-sse/mcp-server/audit.ts open-sse/mcp-server/server.ts src/lib/gracefulShutdown.ts open-sse/mcp-server/__tests__/audit.test.ts./node_modules/.bin/tsc --pretty false -p tsconfig.typecheck-core.json