fix(monitors): truncate create output + add delete confirm

src/lib/crud-commands.ts

@@ -2,7 +2,8 @@ import {Command, Args, Flags, type Interfaces} from '@oclif/core'
 import type {ZodType} from 'zod'
 import {globalFlags, buildClient, display} from './base-command.js'
 import {fetchPaginatedValidated} from './typed-api.js'
-import {apiDelete, apiGetSingle, apiPostSingle, apiPutSingle} from './api-client.js'
+import {apiDelete, apiGetSingle, apiPostSingle, apiPutSingle, type ApiClient} from './api-client.js'
+import {DevhelmAuthError, DevhelmNotFoundError, EXIT_CODES} from './errors.js'
 import type {ColumnDef} from './output.js'
 import {parse as parseSchema} from './response-validation.js'
 import {uuidArg} from './validators.js'
@@ -201,22 +202,110 @@ export function createDeleteCommand<T>(config: ResourceConfig<T>) {
   const idLabel = config.idField ?? 'id'
   class DeleteCmd extends Command {
     static description = `Delete a ${config.name}`
-    static examples = [`<%= config.bin %> ${config.plural} delete <${idLabel}>`]
+    static examples = [
+      `<%= config.bin %> ${config.plural} delete <${idLabel}>`,
+      `<%= config.bin %> ${config.plural} delete <${idLabel}> --yes`,
+    ]
     static args = {[idLabel]: idArg(config)}
-    static flags = {...globalFlags}
+    static flags = {
+      ...globalFlags,
+      yes: Flags.boolean({
+        char: 'y',
+        description: 'Skip the interactive confirmation prompt',
+        default: false,
+      }),
+    }
 
     async run() {
       const {args, flags} = await this.parse(DeleteCmd)
       const client = buildClient(flags)
-      const id = args[idLabel]
-      await apiDelete(client, `${config.apiPath}/${id}`)
+      // The id arg is declared `required: true` on the static args block
+      // above, so oclif guarantees it's a string at runtime. The Record
+      // index signature still types it as optional, hence the narrow.
+      const id = args[idLabel] as string
+      const path = `${config.apiPath}/${id}`
+
+      if (!flags.yes) {
+        if (!process.stdin.isTTY) {
+          // In CI / piped invocations, prompting would hang and silent
+          // auto-confirm would defeat the safety check. Refuse loudly.
+          this.error(
+            `Refusing to delete ${config.name} '${id}' in non-interactive mode without --yes (or -y).`,
+            {exit: EXIT_CODES.VALIDATION},
+          )
+        }
+        const confirmed = await promptForDeletion(config, id, path, client)
+        if (!confirmed) {
+          this.log('Cancelled.')
+          return
+        }
+      }
+
+      await apiDelete(client, path)
       this.log(`${config.name} '${id}' deleted.`)
     }
   }
 
   return DeleteCmd
 }
 
+/**
+ * Best-effort label extractor used by the delete confirmation prompt so
+ * the user sees `'My Monitor' (uuid)` instead of just the bare id. The
+ * candidate keys cover the human-readable identifier on every CRUD
+ * resource we ship: `name` for most, `slug`/`key` for status-page slugs
+ * and secret keys, `summary`/`title` for incidents, `email` for users.
+ * Returns `undefined` when nothing usable is found — the caller falls
+ * back to the id alone, which is still safer than no prompt at all.
+ */
+export function extractEntityLabel(value: unknown): string | undefined {
+  if (!value || typeof value !== 'object') return undefined
+  const record = value as Record<string, unknown>
+  for (const key of ['name', 'slug', 'key', 'title', 'summary', 'email']) {
+    const candidate = record[key]
+    if (typeof candidate === 'string' && candidate.length > 0) return candidate
+  }
+  return undefined
+}
+
+/**
+ * Interactive confirmation prompt for the generated delete commands.
+ *
+ * GETs the resource first so the prompt can show its human-readable
+ * name. A 404 (or 401/403) is surfaced before the destructive action:
+ * the typo'd id never makes it to the prompt. Any other GET failure is
+ * swallowed — we still prompt with the bare id rather than blocking the
+ * user from a delete that would otherwise succeed.
+ *
+ * The non-TTY refusal is done by the caller (it has access to
+ * `this.error()` for a clean oclif exit) so this helper only runs when
+ * stdin is interactive.
+ */
+async function promptForDeletion<T>(
+  config: ResourceConfig<T>,
+  id: string,
+  path: string,
+  client: ApiClient,
+): Promise<boolean> {
+  let label = `'${id}'`
+  try {
+    const value = await apiGetSingle<unknown>(client, path, config.responseSchema as ZodType<unknown>)
+    const name = extractEntityLabel(value)
+    if (name) label = `'${name}' (${id})`
+  } catch (err) {
+    if (err instanceof DevhelmAuthError || err instanceof DevhelmNotFoundError) throw err
+  }
+
+  const {createInterface} = await import('node:readline')
+  const rl = createInterface({input: process.stdin, output: process.stderr})
+  const answer = await new Promise<string>((resolve) => {
+    rl.question(`Delete ${config.name} ${label}? [y/N] `, resolve)
+  })
+  rl.close()
+  const normalized = answer.trim().toLowerCase()
+  return normalized === 'y' || normalized === 'yes'
+}
+
 function extractResourceFlags(flags: Record<string, unknown>, keys: string[]): Record<string, unknown> {
   const body: Record<string, unknown> = {}
   for (const key of keys) {

src/lib/output.ts

@@ -59,12 +59,29 @@ function formatTable(data: unknown, columns?: ColumnDef[]): string {
   return table.toString()
 }
 
+/**
+ * Default-table render of a single record (e.g. the response body of a
+ * `create` / `get` command). Nested objects are JSON-stringified and then
+ * truncated so one noisy field (e.g. `incidentPolicy`, `config`, `auth`)
+ * doesn't blow the value column out to several thousand characters and
+ * push the rest of the record off-screen. Users who actually need the
+ * full structure can re-run with `--output json` (or `yaml`).
+ */
+const SINGLE_RECORD_OBJECT_PREVIEW_CHARS = 80
+const SINGLE_RECORD_TRUNCATION_HINT = '… (use --output json for full)'
+
+function previewObjectValue(value: unknown): string {
+  const json = JSON.stringify(value) ?? ''
+  if (json.length <= SINGLE_RECORD_OBJECT_PREVIEW_CHARS) return json
+  return json.slice(0, SINGLE_RECORD_OBJECT_PREVIEW_CHARS) + SINGLE_RECORD_TRUNCATION_HINT
+}
+
 function formatSingleRecord(data: Record<string, unknown>): string {
   const table = new Table({style: {head: ['cyan']}})
 
   for (const [key, value] of Object.entries(data)) {
     if (value === undefined) continue
-    const display = typeof value === 'object' ? JSON.stringify(value) : String(value ?? '')
+    const display = typeof value === 'object' && value !== null ? previewObjectValue(value) : String(value ?? '')
     table.push({[key]: display})
   }
 

test/lib/delete-confirm.test.ts

@@ -0,0 +1,98 @@
+import {describe, it, expect, beforeEach, afterEach, vi} from 'vitest'
+import {z} from 'zod'
+import {extractEntityLabel, createDeleteCommand, type ResourceConfig} from '../../src/lib/crud-commands.js'
+import {EXIT_CODES} from '../../src/lib/errors.js'
+
+describe('extractEntityLabel', () => {
+  it('returns the name field when present', () => {
+    expect(extractEntityLabel({id: 'x', name: 'My Monitor'})).toBe('My Monitor')
+  })
+
+  it('falls back through the candidate list', () => {
+    expect(extractEntityLabel({id: 'x', slug: 'my-page'})).toBe('my-page')
+    expect(extractEntityLabel({id: 'x', key: 'API_KEY'})).toBe('API_KEY')
+    expect(extractEntityLabel({id: 'x', summary: 'Outage on us-east-1'})).toBe('Outage on us-east-1')
+    expect(extractEntityLabel({id: 'x', email: 'ops@example.com'})).toBe('ops@example.com')
+  })
+
+  it('prefers name over secondary fields', () => {
+    expect(extractEntityLabel({id: 'x', name: 'A', slug: 'b'})).toBe('A')
+  })
+
+  it('returns undefined when no candidate field matches', () => {
+    expect(extractEntityLabel({id: 'x', foo: 'bar'})).toBeUndefined()
+  })
+
+  it('ignores empty strings and non-string values', () => {
+    expect(extractEntityLabel({id: 'x', name: ''})).toBeUndefined()
+    expect(extractEntityLabel({id: 'x', name: 42})).toBeUndefined()
+  })
+
+  it('returns undefined for non-objects', () => {
+    expect(extractEntityLabel(null)).toBeUndefined()
+    expect(extractEntityLabel(undefined)).toBeUndefined()
+    expect(extractEntityLabel('a string')).toBeUndefined()
+    expect(extractEntityLabel(42)).toBeUndefined()
+  })
+})
+
+const TEST_RESOURCE: ResourceConfig<{id: string; name: string}> = {
+  name: 'widget',
+  plural: 'widgets',
+  apiPath: '/api/v1/widgets',
+  responseSchema: z.object({id: z.string(), name: z.string()}),
+  columns: [
+    {header: 'ID', get: (r) => r.id},
+    {header: 'NAME', get: (r) => r.name},
+  ],
+}
+
+describe('createDeleteCommand --yes / TTY behavior', () => {
+  const SAVED_TOKEN = process.env.DEVHELM_API_TOKEN
+  const ORIGINAL_IS_TTY = process.stdin.isTTY
+
+  beforeEach(() => {
+    // Ensure buildClient() doesn't throw on the auth check before we get
+    // to the prompt path the test actually exercises.
+    process.env.DEVHELM_API_TOKEN = 'dh_test_dummy'
+  })
+
+  afterEach(() => {
+    if (SAVED_TOKEN === undefined) delete process.env.DEVHELM_API_TOKEN
+    else process.env.DEVHELM_API_TOKEN = SAVED_TOKEN
+    Object.defineProperty(process.stdin, 'isTTY', {configurable: true, value: ORIGINAL_IS_TTY})
+    vi.restoreAllMocks()
+  })
+
+  it('exposes the --yes / -y flag on the generated command', () => {
+    const Cmd = createDeleteCommand(TEST_RESOURCE) as unknown as {flags: {yes: {char?: string}}}
+    expect(Cmd.flags.yes).toBeDefined()
+    expect(Cmd.flags.yes.char).toBe('y')
+  })
+
+  it('refuses to run in non-TTY mode without --yes', async () => {
+    Object.defineProperty(process.stdin, 'isTTY', {configurable: true, value: false})
+    const Cmd = createDeleteCommand(TEST_RESOURCE)
+    const id = '00000000-0000-0000-0000-000000000001'
+    // oclif's `this.error()` throws a typed Error tagged with the exit
+    // code; we assert on the message + exit code rather than the
+    // concrete class so we're not coupled to oclif's internal type.
+    await expect(Cmd.run([id])).rejects.toThrow(/non-interactive mode without --yes/)
+    await expect(Cmd.run([id])).rejects.toMatchObject({oclif: {exit: EXIT_CODES.VALIDATION}})
+  })
+
+  it('skips the non-TTY refusal entirely when --yes is supplied', async () => {
+    Object.defineProperty(process.stdin, 'isTTY', {configurable: true, value: false})
+    const Cmd = createDeleteCommand(TEST_RESOURCE)
+    const id = '00000000-0000-0000-0000-000000000002'
+    // With --yes we expect the command to bypass the prompt and reach
+    // the apiDelete network call. We don't have a live API so the call
+    // will fail downstream, but it must NOT fail with the
+    // "non-interactive mode" guard.
+    try {
+      await Cmd.run([id, '--yes', '--api-url', 'http://127.0.0.1:1'])
+    } catch (err) {
+      expect(err).not.toMatchObject({message: expect.stringMatching(/non-interactive mode/)})
+    }
+  })
+})

test/lib/output-single-record.test.ts

@@ -0,0 +1,56 @@
+import {describe, it, expect} from 'vitest'
+import {formatOutput} from '../../src/lib/output.js'
+
+describe('formatOutput single-record table', () => {
+  it('JSON-stringifies short nested objects in full', () => {
+    const out = formatOutput({id: 'x', tags: ['a', 'b']}, 'table')
+    // Short arrays/objects render fully — no truncation hint.
+    expect(out).toContain('["a","b"]')
+    expect(out).not.toContain('use --output json')
+  })
+
+  it('truncates long nested object values with a hint to use --output json', () => {
+    const incidentPolicy = {
+      enabled: true,
+      escalations: Array.from({length: 12}, (_, i) => ({
+        order: i,
+        delaySeconds: i * 60,
+        channelIds: ['00000000-0000-0000-0000-000000000000'],
+      })),
+    }
+    const out = formatOutput({id: 'mon-1', name: 'X', incidentPolicy}, 'table')
+
+    // The truncated rendering carries the hint, and the resulting
+    // single-row width stays well under what the raw JSON would produce
+    // (the raw blob is >1000 chars).
+    expect(out).toContain('use --output json')
+    const rawJsonLength = JSON.stringify(incidentPolicy).length
+    expect(rawJsonLength).toBeGreaterThan(500)
+    const widestLine = Math.max(...out.split('\n').map((l) => l.length))
+    expect(widestLine).toBeLessThan(200)
+  })
+
+  it('does not break when a nested object stringifies to exactly the cutoff', () => {
+    // 79-char JSON value — under the limit, no hint expected.
+    const value = {a: 'x'.repeat(70)}
+    expect(JSON.stringify(value).length).toBeLessThan(80)
+    const out = formatOutput({id: 'x', value}, 'table')
+    expect(out).not.toContain('use --output json')
+  })
+
+  it('renders null nested values as empty rather than {}', () => {
+    const out = formatOutput({id: 'x', auth: null}, 'table')
+    // null falls through to `String(value ?? '')` which gives '' — we
+    // never want to render the literal "null" or surface a misleading
+    // truncated "{}" for a missing value.
+    expect(out).not.toContain('null')
+  })
+
+  it('JSON output is unchanged by the table-only truncation logic', () => {
+    const incidentPolicy = {enabled: true, escalations: Array.from({length: 12}, () => ({delaySeconds: 60}))}
+    const out = formatOutput({id: 'mon-1', incidentPolicy}, 'json')
+    // The full structure must round-trip through JSON output even though
+    // the table renderer truncates it.
+    expect(JSON.parse(out)).toEqual({id: 'mon-1', incidentPolicy})
+  })
+})