Skip to content

fix: fully close MCP audit SQLite connections on shutdown - #1349

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.6.8from
rdself:coder/fix-sqlite-wal-shutdown-main
Apr 16, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.6.8from
rdself:coder/fix-sqlite-wal-shutdown-main

Conversation

@rdself

@rdself rdself commented Apr 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • follow-up to fix: close MCP audit SQLite connections on shutdown #1348 on release/v3.6.7
  • keep the MCP audit shutdown fix aligned with the release branch
  • add the standalone-build follow-up so the audit DB handle is shared through globalThis
  • keep Docker stop long enough to finish the shutdown path and preserve regression coverage

Why this PR exists

#1348 fixed the original issue and was merged into release/v3.6.7.

While validating that fix locally in Docker, I found an additional standalone-production detail: the audit module state could be split across different Next.js chunks, so a module-local cache was not reliable enough for shutdown cleanup. This PR is the follow-up on the same release branch and includes that standalone-safe fix.

Root cause

The main SQLite singleton already checkpointed and closed on shutdown, but MCP audit used a separate better-sqlite3 connection that was not part of the shutdown chain.

In standalone production builds, the audit module could also be loaded in different chunks, so a module-local cache was not reliable enough for shutdown cleanup.

Validation

  • ./node_modules/.bin/vitest run --config vitest.mcp.config.ts open-sse/mcp-server/__tests__/audit.test.ts
  • ./node_modules/.bin/eslint open-sse/mcp-server/audit.ts open-sse/mcp-server/__tests__/audit.test.ts
  • ./node_modules/.bin/tsc --pretty false -p tsconfig.typecheck-core.json
  • local Docker verification: exercised MCP/audit access, stopped the container, confirmed shutdown logs emitted both audit DB close and main DB close, and confirmed storage.sqlite-wal / storage.sqlite-shm were removed after normal stop

@rdself
rdself requested a review from diegosouzapw as a code owner April 16, 2026 19:27
Copilot AI review requested due to automatic review settings April 16, 2026 19:27
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an incomplete shutdown path where the MCP audit logger held a separate SQLite connection that wasn’t being closed, which could leave WAL sidecar files behind after container stop (especially in Next.js standalone chunking scenarios).

Changes:

  • Add closeAuditDb() to checkpoint/close the MCP audit SQLite handle and cache it via globalThis for cross-chunk reuse.
  • Invoke audit DB shutdown from both the Next.js graceful shutdown cleanup and the MCP stdio server shutdown path.
  • Add Vitest regression coverage for audit DB close behavior and increase Docker Compose stop grace period.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/lib/gracefulShutdown.ts Adds shutdown-time import + close for MCP audit DB alongside main DB close.
open-sse/mcp-server/server.ts Ensures the MCP stdio server attempts to close the audit DB on exit.
open-sse/mcp-server/audit.ts Moves audit DB handle to globalThis and adds closeAuditDb() with WAL checkpoint + close.
open-sse/mcp-server/tests/audit.test.ts Adds tests validating closeAuditDb() checkpoints/closes and still closes on checkpoint failure.
docker-compose.yml Extends stop grace period to allow shutdown cleanup to complete in Docker.

Comment on lines +99 to 108
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.");
}
Comment on lines +23 to +30
let dbFile: string;

beforeEach(() => {
vi.resetModules();
globalThis.__omnirouteMcpAuditDb = undefined;
dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-mcp-audit-"));
dbFile = path.join(dataDir, "storage.sqlite");
fs.writeFileSync(dbFile, "");
Comment on lines 97 to 108
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.");
}
@rdself
rdself changed the base branch from main to release/v3.6.7 April 16, 2026 19:42
@diegosouzapw
diegosouzapw changed the base branch from release/v3.6.7 to release/v3.6.8 April 16, 2026 19:43
@diegosouzapw
diegosouzapw merged commit bd1ead2 into diegosouzapw:release/v3.6.8 Apr 16, 2026
2 checks passed
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @rdself for this great contribution! 🎉 The fix has been integrated into the release/v3.6.8 branch and will be part of the next release. We appreciate your effort in closing the MCP DB shutdown gap for standalone builds!

@rdself
rdself deleted the coder/fix-sqlite-wal-shutdown-main branch April 20, 2026 05:35
@diegosouzapw diegosouzapw mentioned this pull request Apr 22, 2026
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants