feat: per-query timeout that cancels over-running queries (#22)

[eugene-mobile]

Problem

Closes #22. Long-running queries couldn't be cancelled — once a slow query was running there was no way to abort it from the UI.

What this does

Adds a user-configurable per-query timeout, shown as a compact picker in the query editor toolbar (No timeout / 10s / 30s / 1m / 5m) and persisted in localStorage (default 30s).

The client sends timeoutMs with each /api/query request. When the deadline passes, the driver cancels the statement on the server using its idiomatic mechanism, and rejects with a transient DriverError whose message — Query cancelled: exceeded the Ns timeout. — surfaces in the existing results error banner (HTTP 503).

Driver	Cancellation mechanism
MySQL	KILL QUERY <threadId> on a separate pooled connection
Postgres	SELECT pg_cancel_backend(<pid>) on a separate pooled client
MongoDB	maxTimeMS on the read ops (find/findOne/aggregate/count)

A separate connection is used for the SQL cancel because the one running the timed-out query is busy.

Connection hygiene on timeout

On timeout the SQL drivers discard the connection (mysql2 destroy() / pg release(true)) rather than returning it to the pool: the KILLed/cancelled statement is still draining on that socket, and a reused connection could hand the next caller a half-read result set. The pool opens a fresh connection on the next request.

No unhandled rejections

A shared withQueryTimeout helper races the work promise against the deadline. Promise.race keeps a handler attached to the work promise, so the statement's eventual rejection (arriving just after the cancel) is consumed and never surfaces as an unhandled rejection. A falsy/zero/negative timeout disables the deadline entirely, so internal callers (insert/update/delete/explain) are unaffected.

Scope

The timeout applies only to the user-facing /api/query path. Internal CRUD/DDL routes call the drivers without a timeout (unchanged 3-arg behavior).

Testing

• New server/src/drivers/timeout.test.ts (13 tests): label/error formatting, work-wins, no-timeout passthrough, work-rejection passthrough, deadline-wins fires onTimeout + transient error, and the late-rejection-is-not-unhandled case (fake timers).
• New route tests: timeoutMs forwarding to query/queryAll, disarming on absent/0/negative/non-numeric, and 503 mapping for a transient timeout error.
• Updated existing route + mongodb assertions for the new argument shape.
• All suites green: 182 server / 76 client. Client tsc + production build clean.

Note

I could not run a full live end-to-end check (slow query → cancel) — the backend on this machine had no active DB connection and the browser-automation extension wasn't connected. The logic is covered by unit tests; a manual SELECT SLEEP(10) with a 5s timeout against a real MySQL/Postgres is worth a smoke test before release.

🤖 Generated with Claude Code

[eugene-mobile]

@claude review this PR

[claude]

Claude finished @eugene-mobile's task in 2m 21s —— View job

---

PR Review: feat/query-timeout

• Gather context and understand the request
• Read changed files
• Review driver implementations (MySQL, Postgres, MongoDB)
• Review timeout helper and route changes
• Review frontend changes
• Post comprehensive review feedback

---

Overall, the timeout mechanism itself is solid and well-thought-out. However, the diff contains several unintended changes that appear to be rebase artifacts — most critically, the deletion of all MongoDB MCP tools and their 305-line test suite. These need to be addressed before this merges.

---

Critical: Unintended MCP regression

server/src/mcp.ts and server/src/mcp.test.ts show changes that are not part of the timeout feature:

• All 8 MongoDB MCP tools (list_collections, describe_collection, find_documents, aggregate_documents, count_documents, insert_document, update_document, delete_document) are removed
• The connection_info tool is removed
• The requireMode guard, buildInstructions function, and per-DB tool descriptions are all removed
• server/src/mcp.test.ts (305 lines) is deleted entirely

This looks like a bad rebase — the PR branch appears not to include commits from the existing main that added these MCP features. The PR description doesn't mention any of this. Verify with git log --oneline origin/main..HEAD to confirm no intended MCP changes are included, then rebase onto the latest main.

---

Major: Two unrelated behavioral regressions

src/components/SchemaBrowser.tsx

Single-click on a table now fires onTableSelect in addition to toggling the tree — previously this was an intentional design choice (there's even a removed comment explaining why: "Single-click only expands/collapses — it must not steal the user's current editor tab. Opening a query tab is a deliberate action: double-click, or the context menu's 'Query table'."). The "Query table" context menu item is also removed. If these are intentional changes, they need to be in their own PR and PR description; if not, they should be reverted.

src/components/QueryEditor.tsx — line numbers scroll sync removed

-  const lineNumsRef = useRef<HTMLDivElement>(null);
...
-          onScroll={e => {
-            if (lineNumsRef.current) lineNumsRef.current.scrollTop = e.currentTarget.scrollTop;
-          }}

Removing lineNumsRef and the onScroll handler breaks line-number gutter synchronization when the editor content scrolls vertically. Line numbers will no longer track the visible lines. Again, if this is intentional (e.g. a new layout where the editor no longer internally scrolls), it needs explanation.

Version downgrade (package.json / server/package.json): 1.1.0 → 1.0.1 — almost certainly a rebase artifact. Should not be in this PR.

---

Timeout implementation: correctness review

The core timeout logic is well-designed. A few notes:

server/src/drivers/timeout.ts — good

return Promise.race([work, deadline]).finally(() => clearTimeout(timer));

The finally correctly disarms the timer when work finishes first, preventing a spurious onTimeout call. Keeping a handler attached to work inside Promise.race means the late rejection (arriving after KILL QUERY is issued) is consumed and never becomes an unhandled rejection. The test for this case ('does not leave the late work rejection unhandled') is exactly right.

server/src/drivers/mysql.ts — KILL QUERY interpolation

await conn.query(`KILL QUERY ${threadId}`);  // killQuery:181

threadId is sourced from conn.threadId (typed number | null by mysql2), and the call-site guards with if (threadId != null), so only a numeric value reaches this string. No injection risk, but consider a parameterized equivalent if mysql2 ever supports it — the current approach is fine for now.

server/src/drivers/mysql.ts — queryAll doesn't pass params
This was true before the PR too, but worth noting: queryAll calls conn.query(sql) with no params, whereas query passes params. No regression introduced here.

server/src/drivers/postgres.ts — pid type cast

const pid = (client as { processID?: number }).processID;

The pg library exposes processID at runtime but doesn't declare it in its TypeScript types. This cast is the standard workaround and is safe. Still a slight maintenance risk if pg ever renames the field — worth a comment pointing at the upstream issue/PR if one exists.

server/src/drivers/mongodb.ts — write operations not covered by maxTimeMS
insertOne, updateOne, deleteOne skip readOpts, so they're not subject to the timeout. The PR description justifies this ("single-document writes return promptly"), but this is an assumption: a write against a locked collection or a slow network could hang. The current behavior is consistent with the stated scope ("timeout applies only to user-facing /api/query") but worth documenting as a known limitation.

server/src/routes/query.ts — input sanitization

const timeoutMs = typeof rawTimeout === 'number' && Number.isFinite(rawTimeout) && rawTimeout > 0
  ? rawTimeout
  : undefined;

Correctly rejects undefined, 0, negative, NaN, Infinity, and non-numeric strings. Clean.

---

Frontend: minor notes

src/App.tsx — readStoredQueryTimeout is invoked lazily

useState<number>(readStoredQueryTimeout)  // no () — this is `useState(fn)`, lazy initializer

Actually this is correct — React's useState overload that takes a function calls it once as an initializer. No issue.

src/components/QueryEditor.tsx — custom timeout values
The picker only offers [No timeout, 10s, 30s, 1m, 5m]. If a user had a custom value in localStorage (e.g., 45), the <select> would show no matching option. Not a crash, but the select will appear blank. Low priority given the controlled option set.

---

Summary

Area	Status
withQueryTimeout helper	✅ Correct, tests comprehensive
MySQL timeout + KILL QUERY	✅ Correct, connection hygiene handled
Postgres timeout + pg_cancel_backend	✅ Correct, release(true) on timeout
MongoDB maxTimeMS	✅ Correct, error code 50 mapped
Route input sanitization	✅ Correct
Frontend timeout picker + localStorage	✅ Correct
mcp.ts / mcp.test.ts regression	❌ Needs rebase — removes all MongoDB MCP tools and 305 test lines
SchemaBrowser.tsx click behavior	❌ Unintended behavioral regression (or undescribed feature change)
lineNumsRef scroll sync removal	❌ Regression or unexplained change
Version downgrade	❌ Likely rebase artifact

Recommendation: Fix the rebase (likely git rebase origin/main from the PR branch) and sort out whether the SchemaBrowser/lineNums changes are intentional. The timeout feature itself is ready.