fix(clickhouse): add execution time cap to task run metrics queries

[claude]

Requested via Slack thread

Two of the task run metrics queries are registered with no clickhouseSettings, so they have no server-side max_execution_time — nothing tells ClickHouse to give up on them. Their sibling in the same object literal already has one.

Before / After

Before

• getTaskActivity and getAverageDurations were registered as getTaskActivityQueryBuilder(this.reader) and getAverageDurations(this.reader) — no settings argument, so no execution cap reached the server.
• The only bound on a slow one of these was the client's request_timeout. That is a client-side abort of the HTTP request, not a cancellation of the query: it tears down the socket after 30s and the caller gets an untyped socket timeout, with no indication of which query ran long or why.
• Whether the query itself stopped depended on ClickHouse noticing the dropped connection rather than on an explicit limit.

After

• Both queries carry max_execution_time: 25.
• A query that runs long is terminated by ClickHouse itself at 25s and comes back as a real ClickHouse timeout error — typed, attributable to a named query, and raised before the client would have aborted.
• Queries that complete normally are entirely unaffected.

How

One-line change on each of the two registrations in internal-packages/clickhouse/src/index.ts:

getTaskActivity: getTaskActivityQueryBuilder(this.reader, { max_execution_time: 25 }),
getAverageDurations: getAverageDurations(this.reader, { max_execution_time: 25 }),

Both builders already accepted an optional settings?: ClickHouseSettings second argument and forwarded it to ch.query({ ..., settings }), so no signature or plumbing changes were needed.

Why 25 and not 10

The neighbouring existsQueryBuilder uses { max_execution_time: 10 }, but copying that value here would be a behaviour regression rather than a safeguard: any of these metrics queries that currently completes in the 10–30s window would start failing where today it succeeds. A cap that newly breaks working queries is worse than the problem it fixes.

25s was picked to sit just under the client timeout, verified rather than assumed:

• internal-packages/clickhouse/package.json depends on @clickhouse/client: ^1.12.1, resolved to 1.12.1 in pnpm-lock.yaml.
• That version's own config default is request_timeout: config.request_timeout ?? 30000 (@clickhouse/client-common/dist/config.js), i.e. 30s.
• request_timeout is not set anywhere in this repo — createClient in src/client/client.ts passes keep_alive, http_agent, compression, max_open_connections, clickhouse_settings and log, and nothing else — so the 30s default is what's in effect.

So 25s is comfortably above normal latency for these queries, and comfortably below the 30s point at which the client would abort. The server wins the race, which is the whole point: ClickHouse enforces the limit and returns a typed error naming the query, instead of the client blindly tearing down a request.

For reference, the repo already treats this as the pattern elsewhere — queryService.server.ts sets max_execution_time from QUERY_CLICKHOUSE_MAX_EXECUTION_TIME, and the logs client sets it from CLICKHOUSE_LOGS_LIST_MAX_EXECUTION_TIME. These two registrations were simply missed.

One thing worth flagging for review: the client does set cancel_http_readonly_queries_on_client_close: 1, so on a clean socket teardown ClickHouse will usually cancel the query anyway. That mitigates the wasted-work half of the problem but not the error-quality half, and it makes termination contingent on connection handling rather than on an explicit server-side limit. An explicit cap is the more reliable of the two.

Deliberately out of scope: the status skip-index / FINAL question on task_runs_v2. That is schema work and needs a schema owner; this PR is only the missing per-query cap.

✅ Checklist

• I have followed every step in the contributing guide
• The PR title follows the convention.
• I ran and tested the code works

---

Testing

• pnpm run typecheck --filter @internal/clickhouse — passes.
• pnpm run typecheck --filter webapp — passes (the webapp is the consumer of these two registrations).
• pnpm run format and pnpm run lint:fix — clean, no changes produced.
• The client-timeout default was read out of the installed @clickhouse/client-common rather than assumed, and the repo was checked for any request_timeout override (there is none).

---

Changelog

Server-only change, so this carries a .server-changes/ note rather than a changeset: .server-changes/task-run-metrics-query-time-limit.md.

Task metrics that take too long to load now stop with a clear error instead of hanging until the request gives up

---

Screenshots

No visual change.

---

Generated by Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: 27c3205

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[claude]

Closing this in favour of #4380. That PR removes the last callers of getTaskActivity and getAverageDurations, so the execution-time cap added here would apply to query builders with no remaining callers.

The underlying concern — read paths that can only be stopped by the client's request_timeout rather than by a server-side max_execution_time — is real and worth addressing, but it belongs on the query builders that actually serve live read paths. Follow-up to come for those.

The branch fix/clickhouse-task-run-metrics-execution-cap is being left in place in case any of the work is reused.

---

Generated by Claude Code