fix: don't silently skip unmarked migrations under --table (#36)

.changeset/table-scope-unmarked.md

@@ -0,0 +1,5 @@
+---
+"chkit": patch
+---
+
+`migrate --table` no longer silently skips hand-written migrations that have no `-- operation:` markers. Their target tables can't be determined, so they are now fail-safe **included** (rather than dropped, which left pending work unapplied while appearing successful) and reported — with a warning in human output and an `undeterminedMigrations` array in `--json` output.

apps/docs/src/content/docs/cli/migrate.md

@@ -75,6 +75,8 @@ The journal is written after each migration, not batched. The table is created i
 
 The `--table` flag filters pending migrations to those containing operations targeting the matched tables. Migration SQL files are parsed for `-- operation:` comment markers to determine which tables they affect.
 
+A hand-written migration with no `-- operation:` markers has no determinable target tables. Rather than silently skip it (which would leave pending work unapplied while appearing successful), `--table` **includes** such migrations and prints a warning listing them. In `--json` mode they are reported in an `undeterminedMigrations` array. If you do not want a hand-written migration applied under a scoped run, add an `-- operation: <type> key=table:<db>.<table> risk=<risk>` marker so its scope can be determined.
+
 ### Plugin hooks
 
 The `onBeforeApply` plugin hook runs before each migration is executed and can transform the SQL statements. The `onAfterApply` hook runs after successful execution.

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

@@ -1,3 +1,3 @@
 import { mkdir, readFile } from 'node:fs/promises'
 import { join } from 'node:path'
 
@@ -42,7 +42,7 @@
   run: cmdMigrate,
 }
 
 async function cmdMigrate(
   runCtx: import('../../plugins.js').ChxPluginCommandContext,
 ): Promise<undefined | number> {
   const { flags, config, configPath, pluginRuntime, pluginContext } = runCtx
@@ -111,9 +111,17 @@
     return 0
   }
 
-  const pending = tableScope.enabled
-    ? await filterPendingByScope(migrationsDir, pendingAll, new Set(tableScope.matchedTables))
-    : pendingAll
+  let pending = pendingAll
+  let undeterminedScope: string[] = []
+  if (tableScope.enabled) {
+    const scoped = await filterPendingByScope(
+      migrationsDir,
+      pendingAll,
+      new Set(tableScope.matchedTables),
+    )
+    pending = scoped.inScope
+    undeterminedScope = scoped.undetermined
+  }
 
   if (pending.length === 0) {
     if (jsonMode) {
@@ -125,7 +133,12 @@
   }
 
   if (jsonMode && !executeRequested) {
-    emitJson('migrate', { mode, scope: tableScope, pending })
+    emitJson('migrate', {
+      mode,
+      scope: tableScope,
+      pending,
+      ...(undeterminedScope.length > 0 ? { undeterminedMigrations: undeterminedScope } : {}),
+    })
     return 0
   }
 
@@ -134,6 +147,13 @@
       console.log(`Table scope: ${tableScope.selector ?? ''} (${tableScope.matchCount} matched)`)
       for (const table of tableScope.matchedTables) console.log(`- ${table}`)
     }
+    if (undeterminedScope.length > 0) {
+      console.log(
+        `⚠ ${undeterminedScope.length} pending migration(s) have no table markers; ` +
+          "including them because their target tables can't be determined under --table:",
+      )
+      for (const file of undeterminedScope) console.log(`  - ${file}`)
+    }
     console.log(`Pending migrations: ${pending.length}`)
     for (const file of pending) {
       console.log(`- ${file}`)
@@ -217,7 +237,12 @@
   }
 
   if (jsonMode) {
-    emitJson('migrate', { mode: 'execute', scope: tableScope, applied: appliedNow })
+    emitJson('migrate', {
+      mode: 'execute',
+      scope: tableScope,
+      applied: appliedNow,
+      ...(undeterminedScope.length > 0 ? { undeterminedMigrations: undeterminedScope } : {}),
+    })
     return 0
   }
 

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

@@ -7,19 +7,42 @@ import {
   tableKeyFromOperationKey,
 } from '../../runtime/table-scope.js'
 
+export interface ScopeFilterResult {
+  /** Migrations to apply under the scope (matched + fail-safe-included). */
+  inScope: string[]
+  /**
+   * Migrations included because their target tables could NOT be determined
+   * (no parseable `-- operation:` markers — e.g. hand-written migrations).
+   * Surfaced to the caller so it can warn instead of silently skipping them.
+   */
+  undetermined: string[]
+}
+
 export async function filterPendingByScope(
   migrationsDir: string,
   pending: string[],
   selectedTables: ReadonlySet<string>,
-): Promise<string[]> {
+): Promise<ScopeFilterResult> {
   const selectedDatabases = new Set(
     [...selectedTables].map((table) => table.split('.')[0] ?? ''),
   )
 
   const inScope: string[] = []
+  const undetermined: string[] = []
   for (const file of pending) {
     const sql = await readFile(join(migrationsDir, file), 'utf8')
     const operations = extractMigrationOperationSummaries(sql)
+
+    // A migration with no parseable operation markers (a hand-written one) has
+    // undeterminable target tables. Silently skipping it under --table is a
+    // quiet correctness gap (#36): the user believes all pending work applied.
+    // Fail-safe by including it, and report it so the caller can warn.
+    if (operations.length === 0) {
+      inScope.push(file)
+      undetermined.push(file)
+      continue
+    }
+
     const matches = operations.some((operation) => {
       const tableKey = tableKeyFromOperationKey(operation.key)
       if (tableKey) return selectedTables.has(tableKey)
@@ -33,5 +56,5 @@ export async function filterPendingByScope(
     if (matches) inScope.push(file)
   }
 
-  return inScope
+  return { inScope, undetermined }
 }

packages/cli/src/test/table-scope-unmarked.e2e.test.ts

@@ -0,0 +1,106 @@
+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 {
+  CORE_ENTRY,
+  createJournalTableName,
+  createLiveExecutor,
+  createPrefix,
+  getRequiredEnv,
+  quoteIdent,
+  runCli,
+  waitForTable,
+} from './e2e-testkit.js'
+
+/**
+ * #36: `migrate --apply --table <t>` used to SILENTLY skip hand-written
+ * migrations that carry no operation markers (their target tables can't be
+ * determined), so the user believed all pending work was applied. The fix
+ * fail-safe includes them and reports them as undetermined.
+ */
+describe('@chkit/cli table-scope unmarked migrations e2e (#36)', () => {
+  test('an unmarked migration is applied under --table, not silently skipped', async () => {
+    const liveEnv = getRequiredEnv()
+    const executor = createLiveExecutor(liveEnv)
+    const database = liveEnv.clickhouseDatabase
+    const journalTable = createJournalTableName('scope_unmarked')
+    const prefix = createPrefix('scope_unmarked')
+    const usersTable = `${prefix}_users`
+    const handTable = `${prefix}_hand`
+    const cliEnv = { CHKIT_JOURNAL_TABLE: journalTable, CI: '1' }
+
+    const dir = await mkdtemp(join(tmpdir(), 'chkit-scope-unmarked-'))
+    const config = join(dir, 'clickhouse.config.ts')
+    const migrationsDir = join(dir, 'chkit/migrations')
+    const schemaPath = join(dir, 'schema.ts')
+
+    try {
+      await mkdir(migrationsDir, { recursive: true })
+      await writeFile(
+        schemaPath,
+        `import { schema, table } from '${CORE_ENTRY}'\n\n` +
+          `export default schema(table({\n` +
+          `  database: '${database}',\n` +
+          `  name: '${usersTable}',\n` +
+          `  columns: [{ name: 'id', type: 'UInt64' }],\n` +
+          `  engine: 'MergeTree()',\n` +
+          `  primaryKey: ['id'],\n` +
+          `  orderBy: ['id'],\n` +
+          `}))\n`,
+        'utf8',
+      )
+      await writeFile(
+        config,
+        `export default {\n` +
+          `  schema: '${schemaPath}',\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',
+      )
+
+      // Generated migration (carries `-- operation:` markers for usersTable),
+      // and a hand-written one with NO markers — both pending.
+      const gen = runCli(dir, ['generate', '--config', config, '--name', 'init', '--migration-id', '20990101000000', '--json'], cliEnv)
+      expect(gen.exitCode).toBe(0)
+      const handMigration = '20990101000001_hand_written.sql'
+      await writeFile(
+        join(migrationsDir, handMigration),
+        `CREATE TABLE IF NOT EXISTS ${quoteIdent(database)}.${quoteIdent(handTable)} (id UInt64) ENGINE = MergeTree ORDER BY id;\n`,
+        'utf8',
+      )
+
+      // Scope to the generated table. The hand-written migration has no markers,
+      // so its target table is undetermined — it must still be applied (and
+      // reported), not silently dropped.
+      const migrate = runCli(dir, ['migrate', '--config', config, '--execute', '--table', usersTable, '--json'], cliEnv)
+      expect(migrate.exitCode).toBe(0)
+
+      const payload = JSON.parse(migrate.stdout) as {
+        applied?: Array<{ name: string }>
+        undeterminedMigrations?: string[]
+      }
+      expect(payload.undeterminedMigrations).toContain(handMigration)
+      expect((payload.applied ?? []).map((entry) => entry.name)).toContain(handMigration)
+
+      // The real proof: the hand-written migration's table actually exists.
+      // Before the fix it would have been skipped and never created.
+      await waitForTable(executor, database, handTable)
+    } finally {
+      await rm(dir, { recursive: true, force: true })
+      await executor.command(`DROP TABLE IF EXISTS ${quoteIdent(database)}.${quoteIdent(usersTable)}`)
+      await executor.command(`DROP TABLE IF EXISTS ${quoteIdent(database)}.${quoteIdent(handTable)}`)
+      await executor.command(`DROP TABLE IF EXISTS ${quoteIdent(database)}.${quoteIdent(journalTable)}`)
+      await executor.close()
+    }
+  }, 180_000)
+})