Release 1.18.5#735
Merged
Merged
Conversation
Bumps version from `1.18.4` to `1.18.5` (patch bump). Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
## Summary Removes the `.github/workflows/copilot-setup-steps.yml` workflow (which slowed agent startup by pre-installing deps, building, and booting ClickHouse) and replaces it with a repo-local spec-conformant skill at `.claude/skills/setup/SKILL.md` that the agent can consult on demand when it actually needs to run tests, lints, or type checks. - **`.claude/skills/setup/SKILL.md`** — spec-conformant skill with YAML frontmatter (`name`, `description`), matching the format used by `skills/clickhouse-js-node-troubleshooting/SKILL.md`. Documents: - Node version (`>=20`, `.nvmrc` = 22, CI tests Node 20/22/24). - Installing deps in all three package roots (root workspaces + the independent `examples/node` and `examples/web` packages). - `npm run build` so workspace packages can resolve each other. - `docker compose up -d` for integration tests, with a note that unit tests don't need a server, plus how to override `CLICKHOUSE_VERSION`. - Pointers to the relevant `npm run test:*`, `lint`, `typecheck`, and example scripts. - **Deleted `.github/workflows/copilot-setup-steps.yml`** so the setup job no longer runs before every agent session. ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
## Summary
Drafts a new AI-agent skill at `skills/clickhouse-js-node-coding/`,
sourced from the `examples/node/coding/*.ts` corpus and scoped to
day-to-day coding against `@clickhouse/client` (Node). Mirrors the
structure of the existing `clickhouse-js-node-troubleshooting` skill so
the two compose cleanly.
- **`SKILL.md`** — frontmatter (name + trigger description), Node-only
banner, How-To-Use rules (right method per task, version-constraint
callouts, `JSONEachRow` default, never template-literal user input),
Task Index mapping intent → reference file, and an "Out of scope"
section that delegates only to the existing
`clickhouse-js-node-troubleshooting` skill and routes the other topics
(performance, security, schema-and-deployments, web) to the
corresponding
`examples/node/{performance,security,schema-and-deployments}/` and
`examples/web/` directories instead of pointing at sibling skill names
that don't exist in this repo.
- **`reference/` (11 files)** — one per Task Index row, each opening
with a version-applies banner and citing the backing example(s):
- `client-configuration.md`, `ping.md`, `insert-formats.md`,
`insert-columns.md`, `insert-values.md`, `async-insert.md`,
`select-formats.md`, `query-parameters.md`, `sessions.md`,
`data-types.md`, `custom-json.md`.
- **`evals/evals.json`** — 8 starter evals using the same `{ id, prompt,
expected_output, files, expectations }` shape as the troubleshooting
skill, covering one prompt per major reference file (config, ping,
insert, columns, parameter binding, sessions, JSON column type, custom
JSON parser).
In response to review feedback, the "Configuration via URL parameters"
section in `reference/client-configuration.md` was tightened to spell
out the fixed allowlist (plus the `clickhouse_setting_` / `ch_` /
`http_header_` prefixes) and to call out the `Unknown URL parameters:
...` throw, with a citation to `packages/client-common/src/config.ts`.
The equivalent config-object example in the same file was corrected to
import `ClickHouseLogLevel` from `@clickhouse/client` and use `log: {
level: ClickHouseLogLevel.TRACE }` instead of the invalid top-level
`log_level: 'TRACE'`, with a note clarifying the
URL-parameter-to-config-object mapping for log levels. Additionally,
`reference/select-formats.md` was updated so `ResultSet.close()` is
documented as synchronous (no `await`) in both the methods table row and
the "stream half-consumed" common-pitfall note, matching the actual
`close(): void` API. Following further review, every remaining reference
to the non-existent `clickhouse-js-node-performance` sibling skill was
replaced with a link to the in-repo
[`examples/node/performance/`](https://github.com/ClickHouse/clickhouse-js/tree/main/examples/node/performance)
directory across `SKILL.md`, `reference/async-insert.md`,
`reference/insert-formats.md`, and `reference/select-formats.md` (intro,
methods table, and Parquet-related notes), so the skill no longer points
at unavailable skill identifiers. The "Session-level `SET` commands"
snippet in `reference/sessions.md` was also made self-contained by
adding the `createClient` and `node:crypto` imports, setting
`max_open_connections: 1` alongside `session_id` (consistent with the
per-client concurrency guidance earlier in the same doc), and ending
with `await client.close()`.
No runtime code or build config is touched. The `client-node` `prepack`
script already copies the repo-root `skills/` directory into the
published package, so the new skill ships to npm without further
changes.
## Checklist
- [x] A human-readable description of the changes was provided to
include in CHANGELOG
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
## Summary
Ports `ClickHouse/clickhouse-java/tests/clickhouse-client` to this repo
as `tests/clickhouse-test-runner/`, a small Node CLI that wraps
`@clickhouse/client` to mimic the upstream `clickhouse-client` binary.
With it on `PATH`, the official `tests/clickhouse-test` runner will
exercise the JS client against the core ClickHouse SQL test suite.
- **Standalone package** (`tests/clickhouse-test-runner/`, not a
workspace, only depends on `@clickhouse/client`) with its own
`tsconfig`, eslint, and Vitest config.
- **`bin/clickhouse`** bash shim — short-circuits `extract-from-config
--key listen_host` → `127.0.0.1` (the only key the upstream runner
needs), otherwise `exec node dist/main.js`.
- **Two backends** selected via `CLICKHOUSE_CLIENT_CLI_IMPL` (mirrors
Java's `client`/`jdbc` split):
- `client` (default) — `client.exec({ query, clickhouse_settings: {
default_format: 'TabSeparated', ... } })` with the response stream piped
straight to stdout (raw bytes, no decoding).
- `http` — Node `fetch` against `/?default_format=TabSeparated&...` with
basic auth, body-streamed to stdout with backpressure.
- **Arg parser** (`src/args.ts`) accepting both `--foo bar` and
`--foo=bar`, dashed/underscored variants listed as separate canonical
entries in `BASE_OPTIONS`, short forms (`-h/-u/-d/-q/-s/-n`), and
dynamic long options. Unknown long options are classified against
SERVER/CLIENT_ONLY allowlists copied from the Java port: server settings
are forwarded, client-only and unknown are silently dropped. Per review
feedback, `max_threads` was reclassified from `CLIENT_ONLY_SETTINGS` to
`SERVER_SETTINGS` so `--max_threads=N` from the upstream test runner is
forwarded to ClickHouse instead of being silently dropped. The unused
`aliases` field on `OptionSpec` was removed since the parser never read
it (dash/underscore forms work via separate `BASE_OPTIONS` entries).
- **Quote-aware multi-query splitter** (`src/split-queries.ts`) — tracks
single/double/backtick state and `\`-escapes; only splits on unquoted
`;`.
- **Logging** to `CLICKHOUSE_CLIENT_CLI_LOG` (default
`/tmp/clickhouse-client-cli.log`) with CWD fallback; failures are
swallowed so the CLI never breaks tests.
- **Exit codes** via `process.exitCode` only — never `process.exit()` —
to avoid truncating stdout writes.
- **Vitest unit tests** for `split-queries`, `args`,
`extract-from-config`, and `log`. Live smoke against `docker compose up
-d clickhouse` exercises both backends and `--multiquery` over stdin.
Usage:
```bash
cd tests/clickhouse-test-runner && npm install && npm run build
# Drive the upstream runner (download CH test files; do NOT clone the full repo)
CLICKHOUSE_CLIENT_CLI_LOG=./test-run.log \
PATH="$PWD/tests/clickhouse-test-runner/bin:$PATH" \
/path/to/ClickHouse/tests/clickhouse-test 01428_hash_set_nan_key
```
The SERVER/CLIENT_ONLY allowlists are copied from the Java port and will
need periodic resync as ClickHouse adds settings.
## Checklist
- [x] Unit and integration tests covering the common scenarios were
added
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
…#708) Fix CI errors from #707. - [x] Remove `--print-time` from workflow + README example (it isn't a valid `tests/clickhouse-test` flag) - [x] Remove unused `clickhouse_version` `workflow_dispatch` input from `upstream-sql-tests.yml` - [x] Remove unused `UPSTREAM_REF` env var from `upstream-sql-tests.yml` - [x] Install `jinja2` in `upstream-sql-tests.yml` - [x] Fix `MaxListenersExceededWarning` in `client` backend by replacing `pipeline(result.stream, process.stdout, { end: false })` with `for await (const chunk of result.stream)` so no error/close listeners accumulate on stdout - [x] Keep `--trace-warnings` enabled in `tests/clickhouse-test-runner/bin/clickhouse` so any future Node warnings include their stack traces in CI logs --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
## Summary The `publish` workflow pushes packages to npm but doesn't create a corresponding git tag, leaving the repo's tag history out of sync with published `latest` releases. Updates `.github/workflows/publish.yml` so the manually-dispatched `latest` job tags the released commit after a successful npm publish. The `head` job (auto-publish on every merge to `main`) is intentionally untouched. - **Tagging step**: reads the version from `packages/client-common/package.json`, then creates and pushes an annotated tag in the existing `X.Y.Z` style (e.g. `1.18.2`). Skips idempotently if the tag already exists, so re-running a dispatch is safe. - **Permissions**: scopes `contents: write` to the `latest` job only, preserving the workflow-level least-privilege defaults (`contents: read`, `id-token: write`). ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
## Summary Continues #708 by growing `tests/clickhouse-test-runner/upstream-allowlist.txt` from 1 to 36 upstream ClickHouse stateless SQL tests. Each added test was verified to pass through the harness on both backends (`client`, `http`) and both server matrix versions (`head`, `latest`). Coverage spans: - Basic `SELECT`, parser numerics, `reinterpretAsString` - Array functions, `ARRAY JOIN`, nested columns, `arrayEnumerateUniq` - `GROUP BY` / `WITH TOTALS` / `HAVING`, `DISTINCT` in subqueries - `FixedString` conversions, higher-order function constants, `argMin`/`argMax` - `IN` / set, ORDER BY descending on (Fixed)String - `ANY`/`ALL` `LEFT`/`INNER` joins on numbers, strings, mixed types ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
…t body (#709) - [x] Add a Node.js integration test that streams a RowBinary payload as the request body via `client.exec` (`node_stream_row_binary.test.ts`) - [x] Add a mirror Node.js integration test that reads the same set of values via `client.exec` with `FORMAT RowBinary` and decodes them from the response stream (`node_stream_row_binary_select.test.ts`) - [x] Verify lint, typecheck, and the new test pass against a local ClickHouse --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Continues #708 / #710 by growing `tests/clickhouse-test-runner/upstream-allowlist.txt` from 36 → 424 entries through the Node.js harness, validated on both `client` and `http` backends. - [x] Set up upstream ClickHouse sparse checkout, install Python deps, build the test runner, start ClickHouse via docker-compose - [x] Verify the existing 36-test allowlist passes through the harness on both `client` and `http` backends - [x] Build the per-batch driver (add 100, run both backends with `--no-stateful --no-long`, keep passers, drop failures and skips) - [x] Run batches 1-6 (600 candidates evaluated; 342 passed on both `client` and `http` backends and were kept; 257 failures and 1 skip dropped); allowlist 36 → 378 - [x] Fix CI failures from prior commit: drop 54 entries from the allowlist that either failed in the CI matrix (`{client,http} × {head,latest}`) or that act as **prefix matches** for failing tests (clickhouse-test treats positional args as substring/prefix matches, so e.g. `00396_uuid` was unintentionally pulling in the failing `00396_uuid_v7`). Net: allowlist now 424 entries (+388 vs the pre-#710 baseline of 36) - [x] Document the harness purpose and the batch-add strategy (including the prefix-match gotcha) in `AGENTS.md` so future agent runs have the context - [x] PR ready for review --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
- [x] Document 1.18.4 and 1.18.5 in CHANGELOG.md - [x] Extend `.github/workflows/publish.yml` `latest` job to create the matching GitHub Release from the CHANGELOG section after pushing the tag (idempotent; aborts if the section is missing/empty) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
- [x] Merge `origin/main` into the branch (resolves with `tests/clickhouse-test-runner/upstream-allowlist.txt` auto-merge) - [x] Dedup and sort the allowlist entries (661 entry lines → 481 unique entries, sorted lexicographically; header preserved) - [x] Final allowlist: 481 unique tests, alphabetically sorted --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
`AGENTS.md` had drifted from reality after PRs #662–#712: it claimed `examples/web` has no `performance/` folder, pointed at a placeholder docs path, and said nothing about the `skills/` directory or the `.claude/skills/setup/` skill that replaced `copilot-setup-steps.yml`. ## Summary - **Examples › `performance/`** — drop the "no `performance/` folder under `examples/web`" claim; #701 added one for browser-safe streaming. - **Log-message docs guidance** — point at the real `docs/` layout from #677 (`docs/socket_hang_up_econnreset.md`, `docs/howto/`) instead of a fictional `docs/example-log-message.md`. The example URL in the snippet now also resolves to a real page (`docs/socket_hang_up_econnreset.md`). - **New `Skills` section** — documents the repo-root `skills/` directory (#681, #682, #702): `client-node`'s `prepack` ships it via `@clickhouse/client`, the `agents.skills` field of `packages/client-node/package.json` declares which skills are discoverable, and the `Skills E2E` workflow (`.github/workflows/e2e-skills.yml`, backed by `tests/e2e/skills/check.js`) asserts the packaged tarball contains the declared skills. Routes contributors to `.claude/skills/setup/SKILL.md` (#705) instead of re-introducing `copilot-setup-steps.yml`. - **`agents.skills` manifest** — adds `clickhouse-js-node-coding` alongside `clickhouse-js-node-troubleshooting` so both shipped skills are discoverable to downstream tooling. `tests/e2e/skills/check.js` is extended to verify both skills land in the packaged `@clickhouse/client` tarball. - **New `Embedded docs` section** — surfaces `docs/` as the preferred home for pages linked from log messages. - **CHANGELOG guidance** — aligns with the PR template: entry goes in the PR description and is folded into `CHANGELOG.md` at release time. Documentation refresh plus a small manifest/test change to make the second shipped skill discoverable. ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
Implements a new Node.js–only client option, `max_response_headers_size`, that is forwarded as the `maxHeaderSize` request option to `http.request` / `https.request`. Lifts the per-request response header cap (Node default ~16 KB) without `--max-http-header-size` / `NODE_OPTIONS`. - [x] Add `max_response_headers_size` to `NodeClickHouseClientConfigOptions` and forward it in `NodeConfigImpl.make_connection` - [x] Thread it through `CreateConnectionParams` and `NodeConnectionParams` - [x] Pass it as `maxHeaderSize` in `Http.request` / `Https.request` / custom-agent `*.request` calls - [x] Unit tests covering the new option (config-layer, custom-agent, HTTP and HTTPS connections — set + unset) - [x] Update docs and skills (long-running queries, socket-hangup) to point users at the new option - [x] CHANGELOG entry filed under `1.18.5` - [x] Merge `main` and resolve conflicts in `CHANGELOG.md` and `skills/.../socket-hangup.md` - [x] All change descriptions (CHANGELOG, `docs/howto/long_running_queries.md`, `socket-hangup.md` reference) pin the new option to `>= 1.18.5` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot <copilot@github.com>
…#717) ## Summary Address unresolved review comments from #714 on `tests/e2e/skills/check.js`: the hardcoded skill list and single-skill symlink assertion let a missing/misdeclared skill (or wrong `path`) slip through. Make the repo-root `skills/` directory the source of truth and verify the full `agents.skills` manifest. - **`packages/client-node/package.json`** — add `clickhouse-js-node-coding` to `agents.skills` so both shipped skills are discoverable. (Also landed independently on `main` via #714 and reconciled during the merge.) - **`tests/e2e/skills/check.js`** — discover skills from `skills/` (subdirs containing `SKILL.md`) and assert, for the installed `@clickhouse/client` tarball: - `agents.skills` declares **exactly** that set (sorted-name `deepStrictEqual` — catches both missing and stale entries). - Each declared entry has `path === ./skills/<name>`, the path resolves inside the installed package, and contains `SKILL.md`. - Each declared skill is symlinked under `.claude/skills/` by `skills-npm` and the symlink target contains `SKILL.md` (no longer hardcoded to `clickhouse-js-node-troubleshooting`). - `@clickhouse/client-web` still ships no `skills/` dir. - Descriptive error if `skills/` is missing; `npm-*` link list is re-read inside each check to avoid stale snapshots. - Merged latest `main` and resolved a conflict in `tests/e2e/skills/check.js` by keeping this PR's source-of-truth-driven implementation over the simpler hardcoded `nodeSkills` array introduced by #714. ## Checklist - [x] Unit and integration tests covering the common scenarios were added --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: copilot <copilot@github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
## Summary
The existing `node_response_headers_cap.test.ts` only verifies the
underlying Node.js `http.request` `maxHeaderSize` knob, not that the
client actually wires `max_response_headers_size` through to it. This
adds a sibling integration test that drives the same scenarios via
`createClient` so the public option is covered end-to-end.
- **New test**
`packages/client-node/__tests__/integration/node_response_headers_cap_client.test.ts`:
copy of the original, but the request goes through `createClient({ url,
max_response_headers_size, keep_alive: { enabled: false } })` +
`client.ping()` against the same raw TCP server that emits a
hand-crafted HTTP/1.1 response with a large header block.
- **Assertions**: oversize cases resolve to `{ success: false, error: {
code: 'HPE_HEADER_OVERFLOW' } }`; raised-limit cases resolve to `{
success: true }`. Same six A–F scenarios across the 16K / 32K / 64K
buckets as the original.
- **Original test** `node_response_headers_cap.test.ts` is left
unchanged — it still serves as the pure-Node behavior baseline.
```ts
client = createClient({
url: `http://127.0.0.1:${port}`,
keep_alive: { enabled: false },
max_response_headers_size: 64 * 1024,
})
const result = await client.ping()
```
## Checklist
- [x] Unit and integration tests covering the common scenarios were
added
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
## Summary Update eslint to v10 ## Checklist --------- Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
## Summary
The `upstream-sql-tests` workflow had grown to dominate PR CI runtime
(~170 s for the `Run upstream SQL tests` step in each matrix cell).
Since the allowlist is trivially parallelizable, this splits it across a
third matrix dimension so each shard finishes in roughly a minute.
- **`run-upstream-tests.sh`**: new `SHARD_INDEX` / `SHARD_TOTAL` env
vars. When `SHARD_TOTAL > 1`, the allowlist is filtered round-robin (`i
% SHARD_TOTAL == SHARD_INDEX - 1`). Both inputs are validated as
positive integers with `SHARD_INDEX <= SHARD_TOTAL`. Defaults are `1/1`,
so local invocations are unchanged.
- **`upstream-sql-tests.yml`**: added `shard: [1, 2, 3]` to the matrix
and threaded `SHARD_INDEX: ${{ matrix.shard }}` / `SHARD_TOTAL: 3` into
the run step. Artifact names now include `-shard-N` to avoid upload
collisions.
- **Docs** (`README.md`, `AGENTS.md`): documented the new env vars, the
updated matrix shape, and the rule of thumb to bump `shard` and
`SHARD_TOTAL` together if per-shard runtime climbs back above ~1 minute.
Round-robin (rather than contiguous chunks) keeps per-shard runtimes
even regardless of allowlist ordering. With the current 481 entries the
split is 161 / 160 / 160.
## Checklist
- [x] A human-readable description of the changes was provided to
include in CHANGELOG
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
## Summary Applies review feedback from [#643 (discussion_r3213961603)](#643 (comment)): ESLint v10 requires Node `^20.19.0`, but the root `engines.node` still advertises `>=20`, allowing 20.0–20.18 where ESLint v10 may not run reliably. - Bump root `engines.node` from `>=20` to `>=20.19.0` in `package.json`. Other `package.json` files in the repo are unaffected — `examples/{node,web}` and `tests/clickhouse-test-runner` pin ESLint v9, and `tests/clickhouse-test-runner` already declares `>=20.19.0`. ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
## Summary `tests/clickhouse-test-runner` exposed a `CLICKHOUSE_CLIENT_CLI_IMPL` switch with two backends — `client` (uses `@clickhouse/client`) and `http` (raw `fetch` to port 8123). The `http` backend doesn't exercise any code shipped by this repo; it was carried over from the [`clickhouse-java` harness](https://github.com/ClickHouse/clickhouse-java/tree/main/tests/clickhouse-client), where the same switch toggles between two real product surfaces (`client-v2` vs JDBC). `clickhouse-js` has no such second surface, so the `http` mode is removed entirely along with its CI matrix axis. - **Harness**: deleted `src/backends/http.ts`; `src/main.ts` always dispatches to `executeWithClient`; removed the `CLICKHOUSE_CLIENT_CLI_IMPL` env var, validation, and usage line from `src/main.ts` / `src/args.ts`. - **Runner script**: dropped `CLICKHOUSE_CLIENT_CLI_IMPL` pickup/export from `scripts/run-upstream-tests.sh`. - **CI** (`.github/workflows/upstream-sql-tests.yml`): removed the `impl` matrix axis and corresponding step env; matrix is now `{clickhouse: head | latest} × {shard: 1..3}`, halving the job count. Artifact name no longer includes `${{ matrix.impl }}`. - **Docs**: updated `tests/clickhouse-test-runner/README.md`, `AGENTS.md`, and the `upstream-allowlist.txt` header to drop the dual-backend env var, toggle example, and "must pass on both backends" allowlist rule. ## Checklist - [x] A human-readable description of the changes was provided to include in CHANGELOG Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
### Expand upstream SQL allowlist with 2,157 client-backend tests - [x] Append 2,157 harvested `0_stateless` tests passing on `CLICKHOUSE_CLIENT_CLI_IMPL=client` against ClickHouse `latest` to `tests/clickhouse-test-runner/upstream-allowlist.txt` - [x] Bump the upstream SQL tests matrix from 3 shards to 10 (per review feedback) — `shard` matrix and `SHARD_TOTAL` env updated together in `.github/workflows/upstream-sql-tests.yml` - [x] Remove flaky entries `01660_join_or_inner` and `03653_updating_minmax_idx_after_mutation` from the allowlist - [ ] Follow-up: re-validate against the `http` backend and the `head` ClickHouse image; prune any further entries that don't reproduce on the full CI matrix --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
## Summary
Grows the curated upstream SQL allowlist in
`tests/clickhouse-test-runner/upstream-allowlist.txt` from 2,636 to
3,157 entries (+545 net new tests after dedupe) so the harness covers
more of `ClickHouse/ClickHouse`'s `0_stateless` suite against
`@clickhouse/client`.
- **Candidate selection** — enumerated `0_stateless/*.sql` tests not yet
in the allowlist (~5k) and pre-filtered names tagged for unsupported
infra (`shard`, `replicated`, `zookeeper`, `kafka`, `s3`, `mysql`,
`tls`, …).
- **Batched local run** — executed the candidates in batches of 500
through `scripts/run-upstream-tests.sh --no-stateful --no-long` against
ClickHouse `latest`; kept only `[ OK ]` results (572 tests).
- **Prefix-collision guard** — dropped 12 entries whose names are
substrings of other test files in `queries/0_stateless` (e.g.
`00800_function_java_hash` would pull in
`00800_function_java_hash_with_unsigined_types`), per the rule
documented in `AGENTS.md` and the allowlist header.
- **Concurrency reproducibility** — re-ran the safe set in a single pass
and dropped 15 ALTER/mutation-heavy tests that pass individually but
time out at 30 s under parallel load (e.g.
`01710_projection_optimize_materialize`,
`02662_sparse_columns_mutations_*`,
`03285_materialize_ttl_only_drop_parts`).
- **Out of scope (not in this PR)** — investigation of a representative
failing test for a possible client-side fix is deferred; the run
captured ~1,730 `[ FAIL ]` candidates as triage input for a follow-up
(notable client-suspect surfaces include
`00080_show_tables_and_system_tables`, `00076_ip_coding_functions`,
`00037_totals_limit`, `00063_check_query`).
CI (`upstream-sql-tests.yml`, matrix `{head, latest} × shard 1..10`) is
the source of truth — any newly added test that flakes there should be
removed rather than retried.
## Checklist
- [x] A human-readable description of the changes was provided to
include in CHANGELOG
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prepares release 1.18.5, adds a Node-only response header size option, expands shipped AI-agent skills/docs, and introduces an upstream ClickHouse SQL test harness for validating @clickhouse/client.
Changes:
- Adds
max_response_headers_sizefor Node HTTP/HTTPS requests with tests and documentation. - Adds the
clickhouse-js-node-codingskill, updates skill packaging checks, and expands embedded/reference docs. - Adds a new upstream SQL test runner package/workflow plus release/dependency/workflow updates.
Reviewed changes
Copilot reviewed 64 out of 68 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/e2e/skills/check.js |
Expands skills packaging E2E checks to discover all source skills. |
tests/clickhouse-test-runner/vitest.config.ts |
Adds Vitest config for the upstream runner package. |
tests/clickhouse-test-runner/tsconfig.json |
Adds TypeScript config for runner source/tests. |
tests/clickhouse-test-runner/tsconfig.build.json |
Adds build-specific TypeScript output config. |
tests/clickhouse-test-runner/src/split-queries.ts |
Adds semicolon-based SQL multiquery splitting. |
tests/clickhouse-test-runner/src/settings.ts |
Adds server/client-only setting classification. |
tests/clickhouse-test-runner/src/main.ts |
Adds CLI entrypoint for upstream runner invocations. |
tests/clickhouse-test-runner/src/log.ts |
Adds runner logging helpers. |
tests/clickhouse-test-runner/src/extract-from-config.ts |
Adds shortcut handler for upstream config extraction. |
tests/clickhouse-test-runner/src/backends/client.ts |
Executes parsed SQL through @clickhouse/client. |
tests/clickhouse-test-runner/src/args.ts |
Adds clickhouse-client-compatible argument parsing. |
tests/clickhouse-test-runner/scripts/run-upstream-tests.sh |
Adds helper for building/running allowlisted upstream tests. |
tests/clickhouse-test-runner/README.md |
Documents the upstream SQL runner. |
tests/clickhouse-test-runner/package.json |
Defines the runner package scripts/dependencies. |
tests/clickhouse-test-runner/eslint.config.mjs |
Adds runner ESLint config. |
tests/clickhouse-test-runner/bin/clickhouse |
Adds shell shim for runner invocations. |
tests/clickhouse-test-runner/.gitignore |
Ignores runner build/dependency/upstream artifacts. |
tests/clickhouse-test-runner/__tests__/split-queries.test.ts |
Tests SQL splitting behavior. |
tests/clickhouse-test-runner/__tests__/log.test.ts |
Tests log helper behavior. |
tests/clickhouse-test-runner/__tests__/extract-from-config.test.ts |
Tests config extraction shortcut. |
tests/clickhouse-test-runner/__tests__/args.test.ts |
Tests CLI argument parsing/settings forwarding. |
skills/clickhouse-js-node-troubleshooting/reference/socket-hangup.md |
Updates header-overflow guidance for 1.18.5. |
skills/clickhouse-js-node-coding/SKILL.md |
Adds new coding-focused Node client skill. |
skills/clickhouse-js-node-coding/reference/sessions.md |
Adds sessions/temp-table guidance. |
skills/clickhouse-js-node-coding/reference/select-formats.md |
Adds select format guidance. |
skills/clickhouse-js-node-coding/reference/query-parameters.md |
Adds query parameter guidance. |
skills/clickhouse-js-node-coding/reference/ping.md |
Adds ping/health-check guidance. |
skills/clickhouse-js-node-coding/reference/insert-values.md |
Adds insert values/functions guidance. |
skills/clickhouse-js-node-coding/reference/insert-formats.md |
Adds insert format guidance. |
skills/clickhouse-js-node-coding/reference/insert-columns.md |
Adds partial-column insert guidance. |
skills/clickhouse-js-node-coding/reference/data-types.md |
Adds modern ClickHouse data type guidance. |
skills/clickhouse-js-node-coding/reference/custom-json.md |
Adds custom JSON parse/stringify guidance. |
skills/clickhouse-js-node-coding/reference/client-configuration.md |
Adds client configuration guidance. |
skills/clickhouse-js-node-coding/reference/async-insert.md |
Adds async insert guidance. |
skills/clickhouse-js-node-coding/evals/evals.json |
Adds eval prompts/expectations for the coding skill. |
packages/client-web/src/version.ts |
Bumps web client version to 1.18.5. |
packages/client-web/package.json |
Bumps web package and common dependency versions. |
packages/client-node/src/version.ts |
Bumps Node client version to 1.18.5. |
packages/client-node/src/utils/stream.ts |
Preserves original cause for oversize string conversion errors. |
packages/client-node/src/connection/node_https_connection.ts |
Forwards max_response_headers_size to HTTPS request options. |
packages/client-node/src/connection/node_http_connection.ts |
Forwards max_response_headers_size to HTTP request options. |
packages/client-node/src/connection/node_custom_agent_connection.ts |
Forwards max_response_headers_size with custom agents. |
packages/client-node/src/connection/node_base_connection.ts |
Adds connection param type/docs for response header cap. |
packages/client-node/src/connection/create_connection.ts |
Threads header-size option through connection creation. |
packages/client-node/src/config.ts |
Adds public Node config option and passes it to connections. |
packages/client-node/package.json |
Bumps version/dependency and declares shipped skills. |
packages/client-node/__tests__/unit/node_https_connection.test.ts |
Tests HTTPS request option forwarding. |
packages/client-node/__tests__/unit/node_http_connection.test.ts |
Tests HTTP request option forwarding. |
packages/client-node/__tests__/unit/node_custom_agent_connection.test.ts |
Tests custom-agent request option forwarding. |
packages/client-node/__tests__/unit/node_config.test.ts |
Tests config-to-connection propagation. |
packages/client-node/__tests__/integration/node_stream_row_binary.test.ts |
Adds RowBinary streaming insert integration coverage. |
packages/client-node/__tests__/integration/node_stream_row_binary_select.test.ts |
Adds RowBinary streaming select integration coverage. |
packages/client-node/__tests__/integration/node_response_headers_cap_client.test.ts |
Adds integration coverage for client header-size cap behavior. |
packages/client-common/src/version.ts |
Bumps common package version to 1.18.5. |
packages/client-common/package.json |
Bumps common package version. |
package.json |
Raises root Node engine and updates ESLint dev dependencies. |
package-lock.json |
Updates lockfile for version/dependency changes. |
docs/howto/long_running_queries.md |
Documents Node response header cap option. |
CONTRIBUTING.md |
Links to upstream SQL runner documentation. |
CHANGELOG.md |
Adds release notes for 1.18.5. |
AGENTS.md |
Updates contributor guidance for docs, skills, and upstream tests. |
.github/workflows/upstream-sql-tests.yml |
Adds upstream SQL test CI workflow. |
.github/workflows/publish.yml |
Adds release tag creation and permissions. |
.github/workflows/copilot-setup-steps.yml |
Removes Copilot setup workflow. |
.github/dependabot.yml |
Stops ignoring ESLint 10 updates. |
.claude/skills/setup/SKILL.md |
Adds repository setup skill for agents. |
Files not reviewed (1)
- tests/clickhouse-test-runner/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
A short description of the changes with a link to an open issue.
Checklist
Delete items not relevant to your PR: