fix: polish error messages and noisy outputs

.changeset/error-message-polish.md

@@ -0,0 +1,15 @@
+---
+"chkit": patch
+"@chkit/core": patch
+"@chkit/clickhouse": patch
+"@chkit/plugin-obsessiondb": patch
+---
+
+Polish error messages and a few noisy outputs:
+
+- A table defined with `orderBy` but no `primaryKey` no longer crashes with a raw `TypeError`; the primary key defaults to the order-by columns, matching ClickHouse.
+- Built-in command errors (e.g. a rejected migration) surface their own clean message instead of being wrapped in `Plugin "core" failed in ...`.
+- `chkit query` syntax errors no longer leak the injected `FORMAT JSON` clause the user never typed, and the "Expected one of" token dump is capped.
+- Connection errors whose reason is only in the message (no `.code`) — e.g. a typo'd host — are now recognized and cleaned instead of leaking the raw client string.
+- The post-apply message names the resolved journal table (respecting `CHKIT_JOURNAL_TABLE`) instead of a hardcoded `_chkit_migrations`.
+- The ObsessionDB "authenticated but no service selected" reminder is suppressed when a direct `clickhouse` target is configured (it was layered in from a global login, not chosen for the project).

packages/cli/src/bin/chkit.ts

@@ -127,7 +127,7 @@
   })
 }
 
 async function run(): Promise<void> {
   configureCliLogging()
 
   const argv = process.argv.slice(2)
@@ -201,6 +201,7 @@
     isInteractive: process.stdin.isTTY === true && process.stderr.isTTY === true,
     jsonMode: argv.includes('--json'),
     flags: initFlags,
+    config,
   }
 
   await pluginRuntime.runOnInit(initCtx)

packages/cli/src/commands/migrate/command.ts

@@ -1,3 +1,3 @@
 import { mkdir, readFile } from 'node:fs/promises'
 import { join } from 'node:path'
 
@@ -5,7 +5,7 @@
 import { resolveDirs } from '../../runtime/config.js'
 import { GLOBAL_FLAGS } from '../../runtime/global-flags.js'
 import { emitJson } from '../../runtime/json-output.js'
-import { createJournalStore } from '../../runtime/journal-store.js'
+import { createJournalStore, resolveJournalTableName } from '../../runtime/journal-store.js'
 import { extractMigrationMetadata } from '../../runtime/migration-metadata.js'
 import {
   findChecksumMismatches,
@@ -42,7 +42,7 @@
   run: cmdMigrate,
 }
 
 async function cmdMigrate(
   runCtx: import('../../plugins.js').ChxPluginCommandContext,
 ): Promise<undefined | number> {
   const { flags, config, configPath, pluginRuntime, pluginContext } = runCtx
@@ -221,6 +221,6 @@
     return 0
   }
 
-  console.log('\nMigrations recorded in ClickHouse _chkit_migrations table.')
+  console.log(`\nMigrations recorded in ClickHouse ${resolveJournalTableName()} table.`)
   return 0
 }

packages/cli/src/commands/query.ts

@@ -30,19 +30,57 @@ export const queryCommand: ChxPluginCommand = {
 		}
 
 		if (jsonMode) {
-			const payload = pluginContext.executor.queryJson
-				? await pluginContext.executor.queryJson(sql)
-				: rowsToJsonResult(await pluginContext.executor.query<Record<string, unknown>>(sql))
-			printQueryJson(payload)
-			return 0
+			try {
+				const payload = pluginContext.executor.queryJson
+					? await pluginContext.executor.queryJson(sql)
+					: rowsToJsonResult(await pluginContext.executor.query<Record<string, unknown>>(sql))
+				printQueryJson(payload)
+				return 0
+			} catch (error) {
+				throw cleanQueryError(error)
+			}
 		}
 
-		const rows = await pluginContext.executor.query<Record<string, unknown>>(sql)
-		printRows(rows)
-		return 0
+		try {
+			const rows = await pluginContext.executor.query<Record<string, unknown>>(sql)
+			printRows(rows)
+			return 0
+		} catch (error) {
+			throw cleanQueryError(error)
+		}
 	},
 }
 
+const INJECTED_FORMAT_RE = /\s+FORMAT\s+JSON(?:EachRow)?\b/gi
+const EXPECTED_TOKEN_CAP = 8
+
+/**
+ * ClickHouse syntax errors echo back the query — including the `FORMAT JSON`
+ * clause chkit injects, which the user never typed — and can append a ~40-item
+ * "Expected one of" token dump. Strip the injected FORMAT clause and cap the
+ * token list so the message reflects the SQL the user actually wrote. Returns
+ * the original error untouched when there is nothing to clean (e.g. connection
+ * errors already formatted upstream).
+ */
+export function cleanQueryError(error: unknown): unknown {
+	if (!(error instanceof Error)) return error
+	const original = error.message
+	let message = original.replace(INJECTED_FORMAT_RE, '')
+	message = message.replace(/Expected one of: ([^.]*)\./, (_match, list: string) => {
+		const tokens = list
+			.split(',')
+			.map((token) => token.trim())
+			.filter((token) => token.length > 0)
+		if (tokens.length <= EXPECTED_TOKEN_CAP) return `Expected one of: ${tokens.join(', ')}.`
+		const shown = tokens.slice(0, EXPECTED_TOKEN_CAP).join(', ')
+		return `Expected one of: ${shown}, … (${tokens.length - EXPECTED_TOKEN_CAP} more).`
+	})
+	if (message === original) return error
+	const cleaned = new Error(message.trim())
+	cleaned.stack = error.stack
+	return cleaned
+}
+
 function printQueryJson(payload: ClickHouseJsonQueryResult): void {
 	console.log(formatQueryJson(payload))
 }

packages/cli/src/plugins.ts

@@ -33,6 +33,13 @@ export interface LoadedPlugin {
   options: Record<string, unknown>
   rawOptions: Record<string, unknown>
   plugin: ChxPlugin
+  /**
+   * True for built-in plugins (e.g. `core`) loaded by the CLI itself rather
+   * than from the user's config. Their hook/command errors are surfaced raw,
+   * without the `Plugin "<name>" failed in ...` wrapper that only makes sense
+   * for third-party plugins the user explicitly registered.
+   */
+  internal?: boolean
 }
 
 export interface PluginRuntime {
@@ -166,6 +173,7 @@ export interface ChxOnInitContext {
   isInteractive: boolean
   jsonMode: boolean
   flags: ParsedFlags
+  config: ResolvedChxConfig
   options: Record<string, unknown>
 }
 

packages/cli/src/runtime/journal-store.ts

@@ -36,7 +36,7 @@ export interface JournalStore {
 
 const DEFAULT_JOURNAL_TABLE = '_chkit_migrations'
 
-function resolveJournalTableName(): string {
+export function resolveJournalTableName(): string {
   const candidate = process.env.CHKIT_JOURNAL_TABLE?.trim()
   if (!candidate) return DEFAULT_JOURNAL_TABLE
   if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(candidate)) {

packages/cli/src/runtime/plugin-runtime/index.ts

@@ -41,7 +41,7 @@
   return result.data
 }
 
 export async function loadPluginRuntime(input: {
   config: ResolvedChxConfig
   configPath: string
   cliVersion: string
@@ -88,7 +88,7 @@
   for (const plugin of input.internalPlugins ?? []) {
     if (byName.has(plugin.manifest.name)) continue
     const resolved = resolvePluginOptions(plugin, {})
-    const item: LoadedPlugin = { plugin, options: resolved, rawOptions: {} }
+    const item: LoadedPlugin = { plugin, options: resolved, rawOptions: {}, internal: true }
     loaded.push(item)
     byName.set(plugin.manifest.name, item)
   }
@@ -161,7 +161,7 @@
     runOnCheckReport: (results, print) => runOnCheckReport(byName, results, print),
     runOnBeforePluginCommand: (pluginName, commandName, context) =>
       runOnBeforePluginCommand(loaded, pluginName, commandName, context),
     async runPluginCommand(pluginName, commandName, context) {
       debug('command', `running plugin command "${pluginName}:${commandName}"`)
       const item = byName.get(pluginName)
       if (!item) return 1
@@ -224,6 +224,12 @@
         })
         return typeof code === 'number' ? code : 0
       } catch (error) {
+        // Built-in plugins (core) own the user-facing commands; their errors
+        // should read as the command's own clean message, not as a leaked
+        // internal-plugin failure. Only wrap third-party plugin errors.
+        if (item.internal) {
+          throw error instanceof Error ? error : new Error(String(error))
+        }
         throw formatPluginError(
           item.plugin.manifest.name,
           `command:${commandName}`,

packages/cli/src/test/journal-table-name.e2e.test.ts

@@ -0,0 +1,72 @@
+import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+
+import { describe, expect, test } from 'bun:test'
+
+import {
+  createJournalTableName,
+  createLiveExecutor,
+  createPrefix,
+  getRequiredEnv,
+  quoteIdent,
+  runCli,
+} from './e2e-testkit.js'
+
+describe('@chkit/cli migrate journal-table message e2e (#38)', () => {
+  test('the post-apply message names the resolved journal table, not a hardcoded _chkit_migrations', async () => {
+    const liveEnv = getRequiredEnv()
+    const executor = createLiveExecutor(liveEnv)
+    const database = liveEnv.clickhouseDatabase
+    const journalTable = createJournalTableName('jname')
+    const prefix = createPrefix('jname')
+    const table = `${prefix}t`
+    const dir = await mkdtemp(join(tmpdir(), 'chkit-jname-e2e-'))
+    const migrationsDir = join(dir, 'chkit/migrations')
+    const configPath = join(dir, 'clickhouse.config.ts')
+
+    try {
+      await mkdir(migrationsDir, { recursive: true })
+      await writeFile(
+        configPath,
+        `export default {\n` +
+          `  schema: '${join(dir, 'schema.ts')}',\n` +
+          `  outDir: '${join(dir, 'chkit')}',\n` +
+          `  migrationsDir: '${migrationsDir}',\n` +
+          `  metaDir: '${join(dir, 'chkit/meta')}',\n` +
+          `  clickhouse: {\n` +
+          `    url: '${liveEnv.clickhouseUrl}',\n` +
+          `    username: '${liveEnv.clickhouseUser}',\n` +
+          `    password: '${liveEnv.clickhousePassword}',\n` +
+          `    database: '${database}',\n` +
+          `  },\n}\n`,
+        'utf8',
+      )
+
+      await writeFile(
+        join(migrationsDir, '20990101000000_create.sql'),
+        `CREATE TABLE IF NOT EXISTS ${database}.${table} (id UInt64) ENGINE = MergeTree ORDER BY id;\n`,
+        'utf8',
+      )
+
+      // Non-JSON apply so the human "recorded in ClickHouse <table>" line prints.
+      const result = runCli(dir, ['migrate', '--config', configPath, '--execute'], {
+        CHKIT_JOURNAL_TABLE: journalTable,
+        CI: '1',
+      })
+
+      expect(result.exitCode).toBe(0)
+      expect(result.stdout).toContain(`recorded in ClickHouse ${journalTable} table`)
+      // The default name must NOT be printed when an override is in effect.
+      expect(result.stdout).not.toContain('_chkit_migrations table')
+    } finally {
+      await executor
+        .command(`DROP TABLE IF EXISTS ${quoteIdent(database)}.${quoteIdent(table)}`)
+        .catch(() => {})
+      await executor
+        .command(`DROP TABLE IF EXISTS ${quoteIdent(database)}.${quoteIdent(journalTable)}`)
+        .catch(() => {})
+      await rm(dir, { recursive: true, force: true })
+    }
+  }, 60_000)
+})

packages/cli/src/test/query-clean-error.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, test } from 'bun:test'
+
+import { cleanQueryError } from '../commands/query.js'
+
+describe('cleanQueryError (#24)', () => {
+  test('strips the injected FORMAT JSONEachRow chkit appends to the human query path', () => {
+    const raw = new Error(
+      'Code: 62. DB::Exception: Syntax error: failed at position 13 ((): ( FROM users FORMAT JSONEachRow. Unmatched parentheses: (. (SYNTAX_ERROR)',
+    )
+    const cleaned = cleanQueryError(raw) as Error
+    expect(cleaned.message).not.toContain('FORMAT JSONEachRow')
+    expect(cleaned.message).toContain('( FROM users.')
+  })
+
+  test('strips the injected FORMAT JSON chkit appends to the --json query path', () => {
+    const raw = new Error('Syntax error at position 5: SELECT FORMAT JSON. (SYNTAX_ERROR)')
+    const cleaned = cleanQueryError(raw) as Error
+    expect(cleaned.message).not.toContain('FORMAT JSON')
+  })
+
+  test('caps the "Expected one of" token dump', () => {
+    const raw = new Error(
+      'Syntax error: failed at position 10 (2): 2 3. Expected one of: token, OR, AND, IS NULL, BETWEEN, LIKE, IN, MOD, AS, FROM, WHERE, GROUP BY, ORDER BY, LIMIT, FORMAT, end of query.',
+    )
+    const cleaned = cleanQueryError(raw) as Error
+    expect(cleaned.message).toContain('Expected one of: token, OR, AND, IS NULL, BETWEEN, LIKE, IN, MOD, … (8 more).')
+    // None of the capped-off tokens leak.
+    expect(cleaned.message).not.toContain('GROUP BY')
+  })
+
+  test('returns the original error untouched when there is nothing to clean', () => {
+    const raw = new Error('Could not connect to ClickHouse at https://x (connection refused)')
+    expect(cleanQueryError(raw)).toBe(raw)
+  })
+
+  test('passes non-Error values through unchanged', () => {
+    expect(cleanQueryError('boom')).toBe('boom')
+    expect(cleanQueryError(undefined)).toBe(undefined)
+  })
+})

packages/cli/src/test/query-error.e2e.test.ts

@@ -0,0 +1,48 @@
+import { mkdtemp, rm, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+
+import { describe, expect, test } from 'bun:test'
+
+import { getRequiredEnv, runCli } from './e2e-testkit.js'
+
+describe('@chkit/cli query error surface e2e', () => {
+  test('a SQL syntax error is clean: no "Plugin core failed" wrapper (#20), no injected FORMAT (#24)', async () => {
+    const { clickhouseUrl, clickhouseUser, clickhousePassword, clickhouseDatabase } =
+      getRequiredEnv()
+    const dir = await mkdtemp(join(tmpdir(), 'chkit-query-e2e-'))
+    const configPath = join(dir, 'clickhouse.config.ts')
+    await writeFile(
+      configPath,
+      `export default {\n` +
+        `  schema: '${join(dir, 'schema.ts')}',\n` +
+        `  outDir: '${join(dir, 'chkit')}',\n` +
+        `  migrationsDir: '${join(dir, 'chkit/migrations')}',\n` +
+        `  metaDir: '${join(dir, 'chkit/meta')}',\n` +
+        `  clickhouse: {\n` +
+        `    url: '${clickhouseUrl}',\n` +
+        `    username: '${clickhouseUser}',\n` +
+        `    password: '${clickhousePassword}',\n` +
+        `    database: '${clickhouseDatabase}',\n` +
+        `  },\n}\n`,
+      'utf8',
+    )
+
+    try {
+      const result = runCli(dir, ['query', 'SELECT count( FROM system.one', '--config', configPath], {
+        HOME: dir,
+        XDG_CONFIG_HOME: dir,
+      })
+      expect(result.exitCode).toBe(1)
+      const combined = `${result.stdout}\n${result.stderr}`
+      // The underlying ClickHouse syntax error still surfaces...
+      expect(combined.toLowerCase()).toContain('syntax error')
+      // ...but core is a built-in plugin, so its command error is not wrapped.
+      expect(combined).not.toContain('Plugin "core" failed')
+      // ...and the FORMAT clause chkit injects is stripped (the user never typed it).
+      expect(combined).not.toContain('FORMAT JSON')
+    } finally {
+      await rm(dir, { recursive: true, force: true })
+    }
+  }, 30_000)
+})

packages/clickhouse/src/index.test.ts

@@ -466,6 +466,22 @@ describe('formatConnectionError', () => {
     )
   })
 
+  test('detects a host typo from the message when .code is stripped (#11)', () => {
+    // Some @clickhouse/client / Node versions surface the failure only in the
+    // message with no .code, so the bare-code match misses it and the raw
+    // library string leaks.
+    const error = new Error('getaddrinfo ENOTFOUND db.exampl-typo.com. Was there a typo in the url or port?')
+    expect(formatConnectionError(error, 'https://db.exampl-typo.com:8443')).toBe(
+      'Could not connect to ClickHouse at https://db.exampl-typo.com:8443 (host not found)',
+    )
+  })
+
+  test('detects connection refused from the message alone (no .code)', () => {
+    expect(formatConnectionError(new Error('connect ECONNREFUSED 127.0.0.1:8443'), 'https://x')).toBe(
+      'Could not connect to ClickHouse at https://x (connection refused)',
+    )
+  })
+
   test('returns undefined for unrecognized errors (caller rethrows raw)', () => {
     expect(formatConnectionError(new Error('Unknown data type family: Nope'), 'https://x')).toBeUndefined()
   })