fix: parsing/destructuring command inputs (#130)

* bug fixes

* fix

Author: margaretjgu

AGENTS.md

@@ -85,6 +85,22 @@ When creating or modifying code that constructs URLs, sends credentials, or make
 - **Assert on `RequestInit` configuration in tests**, not just on response output. Verify `redirect`, `method`, and `headers` are set as intended.
 - **Add at least one adversarial input test** for any function that accepts user-provided strings and embeds them in URLs or paths.
 
+## Generic Abstractions Must Handle Real-World Variation
+
+### Lessons
+
+1. **Enumerate all variants a system can produce, not just the ones you see first.** `unwrapField()` only handled `optional` and `default` because those were the only wrappers visible in the initial hand-written schemas. When codegen produced schemas using `z.lazy()`, `z.record()`, `z.any()`, and `z.union()`, they all silently fell through to a catch-all that mapped them to `"string"`. Before writing a generic handler, inspect the full set of types/formats the upstream system can produce and add explicit branches or a loud failure for unrecognized cases.
+
+2. **Fail loudly on unrecognized input instead of falling through to a default.** The `unwrapField` catch-all `return { typeName: def.type, isOptional: false }` silently returned garbage. A `throw new Error('unhandled Zod type: ' + def.type)` would have surfaced the problem immediately at registration time instead of producing subtle runtime validation failures across 1400+ fields.
+
+3. **Test with real generated schemas, not just hand-crafted toy schemas.** If our `extractSchemaArgs` tests had included even one actual schema from the codegen output (which uses `z.lazy` extensively), the bug would have been caught before merging.
+
+4. **Generic request builders need escape hatches for endpoint-specific semantics.** The ES bulk API needs NDJSON; the index API needs body promotion. A "one size fits all" `collectBody()` silently produced wrong output for both. When designing generic abstractions, ask: "what endpoint-specific behavior could this need?" and add explicit extension points (`bodyFormat`, `BODY_ROOT_FIELDS`) rather than special-casing later.
+
+5. **User-facing error messages should diagnose common mistakes.** Raw error propagation (e.g., `SSL routines:tls_get_more_records:packet length too long`) is unhelpful. When you catch an error class that has known causes (TLS mismatch, auth failure, DNS resolution), pattern-match on the message and append a human-readable hint with a suggested fix.
+
+6. **When consuming codegen output, treat it as untrusted input.** The codegen produces valid schemas, but the CLI's generic layers made assumptions about which Zod types would appear. Validate those assumptions with tests that exercise actual generated output.
+
 ## Spec-Kit Workflow
 
 The project uses [spec-kit](https://github.com/github/spec-kit) for AI-assisted feature development.

src/cloud/handler.ts

@@ -9,12 +9,17 @@ import { getCloudClient } from '../lib/cloud-client.ts'
 import { buildCloudRequestParams } from './request-builder.ts'
 import type { JsonValue, ParsedResult } from '../factory.ts'
 
+const DEFAULT_POLL_INTERVAL_MS = 10_000
+const DEFAULT_POLL_TIMEOUT_MS = 300_000
+
 /**
  * Dependencies for `createCloudHandler`. 
  */
 export interface CloudHandlerDeps {
   getCloudClient: () => CloudClient
   buildCloudRequestParams: typeof buildCloudRequestParams
+  pollIntervalMs?: number
+  pollTimeoutMs?: number
 }
 
 const defaultDeps: CloudHandlerDeps = { getCloudClient, buildCloudRequestParams }
@@ -47,13 +52,55 @@ export function createCloudHandler(
 
     try {
       const body = await client.request(params)
+
+      if (parsed.options.wait === true && isCreateProjectCommand(def.name)) {
+        const id = (body as Record<string, unknown>)?.id as string | undefined
+        if (id != null) {
+          const statusPath = `${def.path}/${id}/status`
+          await pollProjectStatus(client, statusPath, deps)
+          process.stderr.write(`Project ${id} is ready.\n`)
+        }
+      }
+
       return body as JsonValue
     } catch (err) {
       return cloudApiError(err)
     }
   }
 }
 
+const CREATE_PROJECT_RE = /^create-(?:elasticsearch|observability|security)-project$/
+
+export function isCreateProjectCommand (name: string): boolean {
+  return CREATE_PROJECT_RE.test(name)
+}
+
+async function pollProjectStatus (
+  client: CloudClient,
+  statusPath: string,
+  deps: CloudHandlerDeps
+): Promise<void> {
+  const interval = deps.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS
+  const timeout = deps.pollTimeoutMs ?? DEFAULT_POLL_TIMEOUT_MS
+  const start = Date.now()
+
+  while (Date.now() - start < timeout) {
+    await sleep(interval)
+    try {
+      const status = await client.request({ method: 'GET', path: statusPath }) as Record<string, unknown>
+      if (status.phase === 'initialized') return
+      process.stderr.write(`Waiting for project... phase: ${status.phase ?? 'unknown'}\n`)
+    } catch {
+      process.stderr.write('Waiting for project... (status check failed, retrying)\n')
+    }
+  }
+  throw new Error('Timed out waiting for project to reach "initialized" phase')
+}
+
+function sleep (ms: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms))
+}
+
 function missingConfigError(err: unknown): JsonValue {
   const message = err instanceof Error ? err.message : String(err)
   return { error: { code: 'missing_config', message } }

src/cloud/register.ts

@@ -4,13 +4,14 @@
  */
 
 import { z } from 'zod'
+import { Command } from 'commander'
 import { defineCommand, defineGroup } from '../factory.ts'
 import type { OpaqueCommandHandle } from '../factory.ts'
 import type { CloudApiDefinition, CloudPathParam, CloudQueryParam } from './types.ts'
 import { validateCloudApiDefinition } from './types.ts'
 import { allCloudApis } from './apis.ts'
 import { allServerlessApis } from './serverless-apis.ts'
-import { createCloudHandler } from './handler.ts'
+import { createCloudHandler, isCreateProjectCommand } from './handler.ts'
 
 /**
  * Builds the unified flat Zod schema for a Cloud API command.
@@ -99,12 +100,16 @@ export function registerCloudCommands(
 
     const leafHandles = defs.map((def) => {
       const schema = buildCommandSchema(def)
-      return defineCommand({
+      const cmd = defineCommand({
         name: def.name,
         description: def.description,
         input: schema,
         handler: createCloudHandler(def),
       })
+      if (isCreateProjectCommand(def.name)) {
+        (cmd as Command).option('--wait', 'Wait for the project to reach "initialized" phase before returning')
+      }
+      return cmd
     })
 
     namespaceHandles.push(

src/es/apis/bulk.ts

@@ -18,6 +18,7 @@ export const bulkApis: EsApiDefinition[] = [
     description: 'Bulk index or delete documents.',
     method: 'POST',
     path: '/{index}/_bulk',
-    input: BulkRequest
+    input: BulkRequest,
+    bodyFormat: 'ndjson'
   }
 ]

src/es/apis/msearch.ts

@@ -18,6 +18,7 @@ export const msearchApis: EsApiDefinition[] = [
     description: 'Run multiple searches.',
     method: 'GET',
     path: '/{index}/_msearch',
-    input: MsearchRequest
+    input: MsearchRequest,
+    bodyFormat: 'ndjson'
   }
 ]

src/es/apis/msearch_template.ts

@@ -18,6 +18,7 @@ export const msearchTemplateApis: EsApiDefinition[] = [
     description: 'Run multiple templated searches.',
     method: 'GET',
     path: '/{index}/_msearch/template',
-    input: MsearchTemplateRequest
+    input: MsearchTemplateRequest,
+    bodyFormat: 'ndjson'
   }
 ]

src/es/errors.ts

@@ -12,6 +12,27 @@ export function missingConfigError (err: unknown): JsonValue {
   return { error: { code: 'missing_config', message } }
 }
 
+const TLS_HINTS = [
+  /SSL routines/i,
+  /wrong version number/i,
+  /ssl3_get_record/i,
+  /tls_get_more_records/i,
+  /packet length too long/i,
+  /EPROTO/i,
+  /ERR_SSL/i,
+]
+
+function isTlsError (message: string): boolean {
+  return TLS_HINTS.some((re) => re.test(message))
+}
+
+function appendTlsHint (message: string): string {
+  if (isTlsError(message)) {
+    return message + '\n\nHint: this looks like a TLS/SSL error. If your Elasticsearch is running on plain HTTP, change the url in your config from https:// to http://.'
+  }
+  return message
+}
+
 /** Builds a structured error payload from a thrown transport error. */
 export function transportError (err: unknown): JsonValue {
   if (err instanceof errors.ResponseError) {
@@ -25,7 +46,7 @@ export function transportError (err: unknown): JsonValue {
   }
 
   if (err instanceof errors.ConnectionError) {
-    return { error: { code: 'connection_error', message: connectionMessage(err) } }
+    return { error: { code: 'connection_error', message: appendTlsHint(connectionMessage(err)) } }
   }
 
   if (err instanceof errors.TimeoutError) {
@@ -34,12 +55,11 @@ export function transportError (err: unknown): JsonValue {
   }
 
   const message = err instanceof Error ? err.message : String(err)
-  return { error: { code: 'transport_error', message } }
+  return { error: { code: 'transport_error', message: appendTlsHint(message) } }
 }
 
 function connectionMessage (err: errors.ConnectionError): string {
   const reason = err.message || 'connection failed'
-  // err.meta is DiagnosticResult; .meta.connection is the nested transport metadata
   const url = err.meta?.meta?.connection?.url?.toString()
   return url ? `${reason} (${url})` : reason
 }

src/es/request-builder.ts

@@ -38,7 +38,14 @@ export function buildRequestParams (
 
   const params: TransportRequestParams = { method: def.method, path }
   if (Object.keys(querystring).length > 0) params.querystring = querystring
-  if (body !== undefined) params.body = body as NonNullable<TransportRequestParams['body']>
+
+  if (body !== undefined) {
+    if (def.bodyFormat === 'ndjson') {
+      params.bulkBody = toNdjson(body)
+    } else {
+      params.body = body as NonNullable<TransportRequestParams['body']>
+    }
+  }
   return params
 }
 
@@ -92,9 +99,35 @@ function buildQuerystring (
   return qs
 }
 
+/**
+ * Serializes a body object into NDJSON format for bulk/msearch APIs.
+ *
+ * Finds the first array-valued field in the body and emits each element as
+ * a separate JSON line. If no array field is found, the body itself is
+ * serialized as a single JSON line. The result always ends with a trailing
+ * newline as required by Elasticsearch.
+ */
+function toNdjson (body: Record<string, unknown>): string {
+  for (const value of Object.values(body)) {
+    if (Array.isArray(value)) {
+      return value.map((item) => JSON.stringify(item)).join('\n') + '\n'
+    }
+  }
+  return JSON.stringify(body) + '\n'
+}
+
+/**
+ * Fields whose value should replace the entire body rather than being nested
+ * under a key. The ES index/create APIs expect the document to BE the body.
+ */
+const BODY_ROOT_FIELDS = new Set(['document'])
+
 /**
  * Collects request body fields from entries with `foundIn === "body"` or no `foundIn`.
  * Returns `undefined` when no body fields are present in the input.
+ *
+ * Special case: when the only body field with a value is in `BODY_ROOT_FIELDS`
+ * (e.g. `document`), its value is promoted to be the entire body (#95).
  */
 function collectBody (
   schemaArgs: SchemaArgDefinition[],
@@ -108,5 +141,12 @@ function collectBody (
     if (value !== undefined) body[arg.schemaKey] = value
   }
 
-  return Object.keys(body).length > 0 ? body : undefined
+  if (Object.keys(body).length === 0) return undefined
+
+  const keys = Object.keys(body)
+  if (keys.length === 1 && BODY_ROOT_FIELDS.has(keys[0]!)) {
+    return body[keys[0]!] as Record<string, unknown>
+  }
+
+  return body
 }

src/es/types.ts

@@ -79,6 +79,8 @@ export interface EsApiDefinition {
   input?: z.ZodObject<z.ZodRawShape> | (() => z.ZodObject<z.ZodRawShape>)
   /** how to handle the response body; defaults to `"json"` */
   responseType?: 'json' | 'text'
+  /** how to serialize the request body; defaults to `"json"` */
+  bodyFormat?: 'json' | 'ndjson'
 }
 
 /** valid command/namespace name: lowercase alphanumeric with hyphens (from `defineCommand` rules) */

src/lib/schema-args.ts

@@ -69,16 +69,16 @@ interface ZodFieldDef {
   type: string
   innerType?: { def: ZodFieldDef }
   defaultValue?: unknown
+  getter?: () => z.ZodType
+  options?: z.ZodType[]
 }
 
 /**
- * Unwraps `optional` and `default` wrapper types from a Zod schema field,
- * returning the underlying type name, optional status, and default value.
+ * Unwraps wrapper types from a Zod schema field, resolving lazy thunks,
+ * records, unions, and any/unknown to their CLI-appropriate type names.
  */
 function unwrapField (field: z.ZodType): { typeName: string, isOptional: boolean, defaultValue?: unknown } {
   const def = field.def as ZodFieldDef
-  // date/bigint/symbol/undefined/void/never cannot be represented in JSON Schema
-  // and would cause z.toJSONSchema() to throw when help is rendered -- fail fast here
   if (def.type === 'date') {
     throw new Error('Date cannot be represented in JSON Schema: use z.string() with an ISO-8601 description instead of z.date()')
   }
@@ -93,6 +93,18 @@ function unwrapField (field: z.ZodType): { typeName: string, isOptional: boolean
     return { ...inner, defaultValue: def.defaultValue, isOptional: false }
   }
 
+  if (def.type === 'lazy' && typeof def.getter === 'function') {
+    return unwrapField(def.getter())
+  }
+
+  if (def.type === 'record' || def.type === 'any' || def.type === 'unknown') {
+    return { typeName: 'object', isOptional: false }
+  }
+
+  if (def.type === 'union' && Array.isArray(def.options) && def.options.length > 0) {
+    return unwrapField(def.options[0] as z.ZodType)
+  }
+
   return { typeName: def.type, isOptional: false }
 }