chore: address review feedback

Author: calvinbrewer

packages/cli/src/commands/encrypt/lib/db-readers.ts

@@ -0,0 +1,114 @@
+import { latestByColumn } from '@cipherstash/migrate'
+import type pg from 'pg'
+
+/**
+ * `latestByColumn` from `@cipherstash/migrate`, but tolerant of the
+ * pre-install case where `cipherstash.cs_migrations` doesn't exist.
+ * The encryption-rollout commands all need to be readable on a fresh
+ * project; treating "table missing" as "no events" keeps them so.
+ */
+export async function latestByColumnSafe(
+  client: pg.ClientBase,
+): Promise<ReturnType<typeof latestByColumn> extends Promise<infer T> ? T : never> {
+  try {
+    return (await latestByColumn(client)) as Awaited<ReturnType<typeof latestByColumn>>
+  } catch (err) {
+    if (
+      err instanceof Error &&
+      /cs_migrations|schema "cipherstash"/i.test(err.message)
+    ) {
+      return new Map() as Awaited<ReturnType<typeof latestByColumn>>
+    }
+    throw err
+  }
+}
+
+export interface EqlColumnInfo {
+  /** Index kinds attached to this column in the EQL config (`unique`,
+   *  `match`, `ore`, `ste_vec`). Empty when no indexes are configured. */
+  indexes: string[]
+  /** Lifecycle state of the EQL config row this column belongs to. */
+  state: 'active' | 'pending' | 'encrypting'
+}
+
+/**
+ * Read every column registered in `eql_v2_configuration` (active,
+ * pending, or encrypting) keyed by `<table>.<column>`. Active rows win
+ * when a column appears in more than one state.
+ *
+ * The call is best-effort: if `eql_v2_configuration` doesn't exist yet
+ * (EQL not installed), an empty map is returned instead of throwing.
+ */
+export async function fetchActiveEqlConfig(
+  client: pg.ClientBase,
+): Promise<Map<string, EqlColumnInfo>> {
+  const out = new Map<string, EqlColumnInfo>()
+  try {
+    const result = await client.query<{ state: string; data: unknown }>(
+      `SELECT state, data FROM public.eql_v2_configuration
+       WHERE state IN ('active', 'pending', 'encrypting')
+       ORDER BY CASE state WHEN 'active' THEN 0 WHEN 'encrypting' THEN 1 ELSE 2 END`,
+    )
+    for (const row of result.rows) {
+      const data = row.data as {
+        tables?: Record<
+          string,
+          Record<string, { indexes?: Record<string, unknown> }>
+        >
+      } | null
+      if (!data?.tables) continue
+      for (const [tableName, columns] of Object.entries(data.tables)) {
+        for (const [columnName, column] of Object.entries(columns)) {
+          const key = `${tableName}.${columnName}`
+          if (out.has(key)) continue
+          out.set(key, {
+            indexes: Object.keys(column.indexes ?? {}),
+            state: row.state as 'active' | 'pending' | 'encrypting',
+          })
+        }
+      }
+    }
+  } catch (err) {
+    if (err instanceof Error && /eql_v2_configuration/i.test(err.message)) {
+      return out
+    }
+    throw err
+  }
+  return out
+}
+
+/**
+ * Read `information_schema.columns` and group column names by table.
+ * When `tables` is provided the query is constrained to that set —
+ * status's quest log only ever needs ~5 specific tables, so passing
+ * the manifest's tables avoids a full-schema scan.
+ */
+export async function fetchPhysicalColumns(
+  client: pg.ClientBase,
+  tables?: ReadonlyArray<string>,
+): Promise<Map<string, Set<string>>> {
+  const out = new Map<string, Set<string>>()
+  try {
+    const result =
+      tables === undefined
+        ? await client.query<{ table_name: string; column_name: string }>(
+            `SELECT table_name, column_name FROM information_schema.columns
+             WHERE table_schema = current_schema()`,
+          )
+        : await client.query<{ table_name: string; column_name: string }>(
+            `SELECT table_name, column_name FROM information_schema.columns
+             WHERE table_schema = current_schema()
+               AND table_name = ANY($1::text[])`,
+            [tables],
+          )
+    for (const row of result.rows) {
+      const set = out.get(row.table_name) ?? new Set<string>()
+      set.add(row.column_name)
+      out.set(row.table_name, set)
+    }
+  } catch {
+    // information_schema is always present; failures here are surprising
+    // enough to swallow rather than crash the read-only status path.
+  }
+  return out
+}

packages/cli/src/commands/encrypt/status.ts

@@ -1,12 +1,14 @@
 import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js'
 import { loadStashConfig } from '@/config/index.js'
-import {
-  type MigrationPhase,
-  latestByColumn,
-  readManifest,
-} from '@cipherstash/migrate'
+import { type MigrationPhase, readManifest } from '@cipherstash/migrate'
 import * as p from '@clack/prompts'
 import pg from 'pg'
+import {
+  type EqlColumnInfo,
+  fetchActiveEqlConfig,
+  fetchPhysicalColumns,
+  latestByColumnSafe,
+} from './lib/db-readers.js'
 
 interface Row {
   table: string
@@ -53,7 +55,7 @@ export async function statusCommand() {
     if (manifest) {
       for (const [tableName, columns] of Object.entries(manifest.tables)) {
         for (const column of columns) {
-          const key = `${tableName}.${column.column}`
+          const key: `${string}.${string}` = `${tableName}.${column.column}`
           seen.add(key)
           rows.push(
             renderRow({
@@ -110,82 +112,6 @@ export async function statusCommand() {
   if (exitCode) process.exit(exitCode)
 }
 
-async function latestByColumnSafe(client: pg.Client) {
-  try {
-    return await latestByColumn(client)
-  } catch (err) {
-    if (
-      err instanceof Error &&
-      /cs_migrations|schema "cipherstash"/i.test(err.message)
-    ) {
-      return new Map()
-    }
-    throw err
-  }
-}
-
-interface EqlColumnInfo {
-  indexes: string[]
-  state: 'active' | 'pending' | 'encrypting'
-}
-
-async function fetchActiveEqlConfig(
-  client: pg.Client,
-): Promise<Map<string, EqlColumnInfo>> {
-  const out = new Map<string, EqlColumnInfo>()
-  try {
-    const result = await client.query<{ state: string; data: unknown }>(
-      `SELECT state, data FROM public.eql_v2_configuration
-       WHERE state IN ('active', 'pending', 'encrypting')
-       ORDER BY CASE state WHEN 'active' THEN 0 WHEN 'encrypting' THEN 1 ELSE 2 END`,
-    )
-    for (const row of result.rows) {
-      const data = row.data as {
-        tables?: Record<
-          string,
-          Record<string, { indexes?: Record<string, unknown> }>
-        >
-      } | null
-      if (!data?.tables) continue
-      for (const [tableName, columns] of Object.entries(data.tables)) {
-        for (const [columnName, column] of Object.entries(columns)) {
-          const key = `${tableName}.${columnName}`
-          if (out.has(key)) continue
-          out.set(key, {
-            indexes: Object.keys(column.indexes ?? {}),
-            state: row.state as 'active' | 'pending' | 'encrypting',
-          })
-        }
-      }
-    }
-  } catch (err) {
-    if (err instanceof Error && /eql_v2_configuration/i.test(err.message)) {
-      return out
-    }
-    throw err
-  }
-  return out
-}
-
-async function fetchPhysicalColumns(
-  client: pg.Client,
-): Promise<Map<string, Set<string>>> {
-  const out = new Map<string, Set<string>>()
-  const result = await client.query<{
-    table_name: string
-    column_name: string
-  }>(
-    `SELECT table_name, column_name FROM information_schema.columns
-     WHERE table_schema = current_schema()`,
-  )
-  for (const row of result.rows) {
-    const set = out.get(row.table_name) ?? new Set<string>()
-    set.add(row.column_name)
-    out.set(row.table_name, set)
-  }
-  return out
-}
-
 function renderRow(input: {
   tableName: string
   columnName: string

packages/cli/src/commands/impl/index.ts

@@ -85,6 +85,7 @@ async function verifyCutoverPreconditions(
   }
 
   const states = await detectColumnStates(databaseUrl, migrate)
+  if (states === null) return null
   return states
     .filter((s) => classifyPhase(s.phase) !== 'cutover')
     .map((s) => ({ table: s.table, column: s.column }))
@@ -122,30 +123,13 @@ function printDeployGateBanner(cli: string): void {
 }
 
 /**
- * `stash impl` — execute an encryption plan.
+ * `stash impl` — execute an encryption plan via agent handoff.
  *
- * Always runs in implement mode. Behaviour branches on disk state and
- * flags:
- *
- *   - **Plan exists** (TTY): parse the structured summary block, render
- *     a confirmation panel, ask the user to proceed. Default-yes.
- *     For `cutover`-step plans, verify `dual_writing` events are
- *     recorded for every migrate column before launching — refuse if
- *     not, and point the user at re-running `stash plan` after deploy.
- *   - **Plan exists** (non-TTY): proceed without confirmation.
- *   - **No plan, `--continue-without-plan`**: confirm once, then implement.
- *   - **No plan, TTY**: present a `p.select` — draft a plan first
- *     (delegates to `planCommand`) or continue without one (confirms
- *     once, then implements).
- *   - **No plan, non-TTY**: error out with a clear next-action; CI must
- *     pass `--continue-without-plan` or run `stash plan` first.
- *
- * After successful handoff, the outro depends on plan step:
- *   - `rollout`  — deploy-gate banner; explicit "do not run encrypt
- *                  backfill yet" message.
- *   - `cutover`  — confirmation that the rollout is fully complete.
- *   - `complete` — same as cutover (escape hatch covers everything).
- *   - no plan / no summary — generic "verify state" pointer.
+ * Cutover-step plans are gated on a `dual_writing` event being recorded
+ * in `cs_migrations` for every migrate column (the safety net that
+ * stops destructive work running ahead of a production deploy of the
+ * dual-write code). After a rollout-step handoff, the outro is the
+ * deploy-gate banner instead of a generic "verify state" pointer.
  */
 export async function implCommand(flags: Record<string, boolean>) {
   const cwd = process.cwd()
@@ -200,7 +184,7 @@ export async function implCommand(flags: Record<string, boolean>) {
       // (potentially hour-long) implementation phase.
       if (isTTY) {
         if (summary) {
-          p.note(renderPlanSummary(summary), 'Plan summary')
+          p.note(renderPlanSummary(summary, cli), 'Plan summary')
         } else {
           p.note(
             `Plan at \`${PLAN_REL_PATH}\` doesn't include a machine-readable summary. Open it in your editor before proceeding.`,

packages/cli/src/commands/impl/steps/handoff-wizard.ts

@@ -12,10 +12,12 @@ import { runWizardSpawn } from '../../wizard/index.js'
  * Hand off to the CipherStash Agent (the in-house wizard package).
  *
  * Writes `.cipherstash/context.json` so the wizard has the same prepared
- * facts the other handoffs use, then spawns the wizard via `runWizardSpawn`
- * — the same path the top-level `stash wizard` subcommand takes, but with
- * the exit code surfaced rather than `process.exit`-ed so `stash impl` can
- * finish its own outro.
+ * facts the other handoffs use (including the resolved `planStep` when
+ * `stash plan` is the caller — the wizard reads it from there rather
+ * than via argv), then spawns the wizard via `runWizardSpawn` — the same
+ * path the top-level `stash wizard` subcommand takes, but with the exit
+ * code surfaced rather than `process.exit`-ed so `stash impl` can finish
+ * its own outro.
  *
  * No skills are installed here. The wizard fetches its own agent-side
  * prompt from the gateway and runs its own `maybeInstallSkills` flow.