v0.17 — contract hardening (exit 64 + piped JSON errors)

CHANGELOG.md

@@ -4,6 +4,38 @@ All notable changes to `pdcli` are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/); versions follow
 [SemVer](https://semver.org/).
 
+## [0.17.0] - 2026-06-11
+
+Contract-hardening release — the last changes to the machine-facing surface
+(exit codes + error output) before the 1.0 stability freeze.
+
+### Changed
+
+- **BREAKING — usage errors now exit 64, not 70.** Malformed invocations
+  (unknown flag, missing argument, invalid flag value) are oclif parse errors;
+  they previously fell through to exit 70 (`EX_SOFTWARE`, "internal bug") and
+  now correctly exit 64 (`EX_USAGE`) per the sysexits ladder. Exit 70 is now
+  reserved for genuinely unexpected internal failures. Scripts branching on the
+  exit code should treat 64 as "fix your command line".
+- **BREAKING — piped errors emit JSON.** Error output now follows the same
+  format resolution as success output: human (`table`) in a TTY, but a JSON
+  envelope (`{ error, message, exitCode, … }`) on stderr whenever output is a
+  machine format — `--output json|yaml|csv`, a non-`table` profile
+  `default_output`, **or when stdout is piped**. Previously a piped run emitted
+  JSON on success but human text on failure; a non-interactive consumer now
+  always gets a parseable error. Pass `--output table` to force human errors.
+
+### Fixed
+
+- OAuth: a token that expired _during_ a 429 backoff is now refreshed. The
+  refresh was gated on the first attempt, so a rate-limit retry consumed the
+  window and the later 401 failed unrecoverably; the refresh is now a single
+  free round independent of the retry budget (works under `--no-retry` too).
+- CSV/`--field`: a field definition missing `field_name` no longer throws when
+  matching by hash code (null-guarded, matching the other resolvers).
+- Error reporting no longer crashes if reading the profile's `default_output`
+  throws (e.g. a corrupt config) while formatting another error.
+
 ## [0.16.0] - 2026-06-11
 
 ### Added

docs/commands.md

@@ -5,7 +5,7 @@ description: Full command reference for the pdcli command-line interface.
 
 <!-- AUTO-GENERATED from the oclif manifest by scripts/gen-commands.mjs — do not edit by hand. -->
 
-Reference for `pdcli` v0.16.0 (145 commands). Every command also accepts the global flags `--output table|json|yaml|csv`, `--profile`, `--no-color`, `--verbose`, `--no-retry`, `--timeout`, and `--limit`.
+Reference for `pdcli` v0.17.0 (145 commands). Every command also accepts the global flags `--output table|json|yaml|csv`, `--profile`, `--no-color`, `--verbose`, `--no-retry`, `--timeout`, and `--limit`.
 
 ## Top-level
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@wavyx/pdcli",
-  "version": "0.16.0",
+  "version": "0.17.0",
   "publishConfig": {
     "access": "public"
   },

src/base-command.js

@@ -130,7 +130,14 @@ export default class BaseCommand extends Command {
    * when `this.flags` is still undefined).
    */
   storedDefaultOutput() {
-    const stored = loadConfig(this.flags?.profile).default_output
+    let stored
+    try {
+      stored = loadConfig(this.flags?.profile).default_output
+    } catch {
+      // The error handler consults this while reporting another failure — a
+      // broken/unreadable config must never crash error reporting itself.
+      return undefined
+    }
     return ['table', 'json', 'yaml', 'csv'].includes(stored)
       ? stored
       : undefined

src/lib/client.js

@@ -163,6 +163,7 @@ export function createClient({
     const maxAttempts = retry ? 3 : 1
     let attempts = 0
     let sawRateLimit = false
+    let refreshed = false
 
     while (attempts < maxAttempts) {
       attempts++
@@ -214,10 +215,15 @@ export function createClient({
         )
       }
 
-      // OAuth access tokens expire (~1h) — refresh once and retry.
-      if (res.status === 401 && onRefresh && attempts === 1) {
+      // OAuth access tokens expire (~1h) — refresh once and retry. Gate on a
+      // dedicated flag, NOT attempts===1: a 429 backoff can consume the early
+      // attempts, so the expiring-token 401 may not arrive until later. The
+      // refresh round is free (attempts--) so it never eats the retry budget.
+      if (res.status === 401 && onRefresh && !refreshed) {
         debug('401, attempting OAuth token refresh')
+        refreshed = true
         token = await onRefresh()
+        attempts--
         continue
       }
 

src/lib/errors.js

@@ -84,19 +84,27 @@ export class ApiError extends CliError {
  * @param {import('@oclif/core').Command} cmd
  */
 export function handleError(err, cmd) {
-  const exitCode = err.exitCode ?? 70
+  // oclif parse/usage errors (unknown flag, missing arg, bad enum) carry
+  // `oclif.exit === 2` and no `exitCode` of ours — they're a USAGE problem
+  // (64), not an internal CLI bug (70). Everything else: our exitCode, else 70.
+  const isUsageError = err.exitCode == null && err.oclif?.exit === 2
+  const exitCode = isUsageError ? 64 : (err.exitCode ?? 70)
   const flags = cmd.flags ?? {}
 
-  // JSON errors when the user asked for JSON — via the flag or the
-  // profile's default_output. (The piped-TTY fallback intentionally does
-  // NOT apply here; errors stay human unless JSON was requested.)
-  const format =
-    flags.output ??
-    (typeof cmd.storedDefaultOutput === 'function'
+  // Resolve the format like success output (resolveFormat): explicit flag,
+  // else the profile default, else JSON when piped / 'table' in a TTY. Any
+  // non-table format (json/yaml/csv, or piped) gets the JSON error envelope —
+  // there is no yaml/csv error serializer, and a machine consumer that gets
+  // JSON on success must get a parseable error on failure. Only an interactive
+  // 'table' context stays human.
+  const stored =
+    typeof cmd.storedDefaultOutput === 'function'
       ? cmd.storedDefaultOutput()
-      : undefined)
+      : undefined
+  const format =
+    flags.output ?? stored ?? (process.stdout.isTTY ? 'table' : 'json')
 
-  if (format === 'json') {
+  if (format !== 'table') {
     const payload = {
       error: err.constructor.name,
       message: err.message,

src/lib/input.js

@@ -78,7 +78,7 @@ function isHashKey(s) {
 function findField(defs, name) {
   const lower = name.toLowerCase()
   return defs.find(
-    (d) => d.field_name.toLowerCase() === lower || d.field_code === name,
+    (d) => d.field_name?.toLowerCase() === lower || d.field_code === name,
   )
 }
 

test/base-command.test.js

@@ -115,7 +115,13 @@ describe('BaseCommand', () => {
 
   it('propagates AuthRequiredError when credentials are missing', async () => {
     mockResolveCredentials.mockRejectedValue(new AuthRequiredError())
-    await expect(captureLogs(ApiCmd)).rejects.toThrow('Not authenticated')
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = true // interactive → human error carries the message
+    try {
+      await expect(captureLogs(ApiCmd)).rejects.toThrow('Not authenticated')
+    } finally {
+      process.stdout.isTTY = origIsTTY
+    }
   })
 
   it('defaults to json output when not TTY', async () => {
@@ -159,9 +165,15 @@ describe('BaseCommand', () => {
       .get('/api/v2/users/me')
       .reply(422, { success: false, error: 'Validation failed' })
 
-    await expect(captureLogs(ApiCmd)).rejects.toThrow(
-      'Pipedrive API 422: Validation failed',
-    )
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = true // interactive → human error carries the message
+    try {
+      await expect(captureLogs(ApiCmd)).rejects.toThrow(
+        'Pipedrive API 422: Validation failed',
+      )
+    } finally {
+      process.stdout.isTTY = origIsTTY
+    }
   })
 
   it('sends a pdcli User-Agent on API requests', async () => {
@@ -185,9 +197,15 @@ describe('BaseCommand', () => {
       .get('/api/v2/users/me')
       .reply(429, '', { 'x-ratelimit-reset': '9' })
 
-    await expect(captureLogs(ApiCmd, ['--no-retry'])).rejects.toThrow(
-      /Rate limited/,
-    )
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = true // interactive → human error carries the message
+    try {
+      await expect(captureLogs(ApiCmd, ['--no-retry'])).rejects.toThrow(
+        /Rate limited/,
+      )
+    } finally {
+      process.stdout.isTTY = origIsTTY
+    }
   })
 
   it('defines a --resolve-fields global flag for non-table formats', () => {
@@ -410,9 +428,14 @@ describe('resolveFormat with default_output', () => {
       activeProfile: 'default',
       default_output: 'bogus',
     })
-    const stdout = await captureLogs(FormatCmd, [])
-    // vitest runs piped (non-TTY), so the fallback is json
-    expect(stdout).toContain('format:json')
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = false // piped → the fallback is json
+    try {
+      const stdout = await captureLogs(FormatCmd, [])
+      expect(stdout).toContain('format:json')
+    } finally {
+      process.stdout.isTTY = origIsTTY
+    }
   })
 
   it('emits JSON errors when default_output is json (no --output flag)', async () => {
@@ -448,29 +471,143 @@ describe('resolveFormat with default_output', () => {
     // Before the fix this died with "Cannot read properties of undefined
     // (reading 'profile')" because handleError consulted
     // storedDefaultOutput() while this.flags was still undefined.
-    await expect(ParseCmd.run(['--no-such-flag'])).rejects.toThrow(
-      /Nonexistent flag/,
-    )
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = true // interactive → human error carries the message
+    try {
+      await expect(ParseCmd.run(['--no-such-flag'])).rejects.toThrow(
+        /Nonexistent flag/,
+      )
+    } finally {
+      process.stdout.isTTY = origIsTTY
+    }
+  })
+
+  it('maps oclif parse errors to exit 64 (usage), not 70 (internal)', async () => {
+    mockLoadConfig.mockReturnValue({ activeProfile: 'default' })
+    class ParseCmd extends BaseCommand {
+      static skipAuth = true
+      async run() {}
+    }
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = true // human path
+    const err = await ParseCmd.run(['--no-such-flag']).catch((e) => e)
+    process.stdout.isTTY = origIsTTY
+    expect(err.exitCode ?? err.oclif?.exit).toBe(64)
+  })
+
+  it('emits a JSON parse error (exit 64) when piped', async () => {
+    mockLoadConfig.mockReturnValue({ activeProfile: 'default' })
+    class ParseCmd extends BaseCommand {
+      static skipAuth = true
+      async run() {}
+    }
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = false // piped
+    const writes = []
+    const spy = vi.spyOn(process.stderr, 'write').mockImplementation((c) => {
+      writes.push(String(c))
+      return true
+    })
+    await ParseCmd.run(['--no-such-flag']).catch(() => {})
+    spy.mockRestore()
+    process.stdout.isTTY = origIsTTY
+    const payload = JSON.parse(writes.join(''))
+    expect(payload.exitCode).toBe(64)
+    expect(payload.message).toMatch(/nonexistent flag/i)
   })
 
-  it('keeps human-form errors when no flag and no stored default', async () => {
+  it('emits JSON errors when piped, even with no flag or stored default', async () => {
     mockLoadConfig.mockReturnValue({ activeProfile: 'default' })
     class FailCmd extends BaseCommand {
       static skipAuth = true
       async run() {
         throw new AuthRequiredError()
       }
     }
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = false // piped → machine consumer
     const writes = []
-    const spy = vi
-      .spyOn(process.stderr, 'write')
-      .mockImplementation((chunk) => {
-        writes.push(String(chunk))
-        return true
-      })
-    // piped (non-TTY) but no explicit intent → human error, not JSON
+    const spy = vi.spyOn(process.stderr, 'write').mockImplementation((c) => {
+      writes.push(String(c))
+      return true
+    })
+    await expect(FailCmd.run([])).rejects.toThrow()
+    spy.mockRestore()
+    process.stdout.isTTY = origIsTTY
+    const payload = JSON.parse(writes.join(''))
+    expect(payload.error).toBe('AuthRequiredError')
+    expect(payload.exitCode).toBe(77)
+  })
+
+  it('keeps human (non-JSON) errors in a TTY with no flag or stored default', async () => {
+    mockLoadConfig.mockReturnValue({ activeProfile: 'default' })
+    class FailCmd extends BaseCommand {
+      static skipAuth = true
+      async run() {
+        throw new AuthRequiredError()
+      }
+    }
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = true // interactive terminal
+    const writes = []
+    const spy = vi.spyOn(process.stderr, 'write').mockImplementation((c) => {
+      writes.push(String(c))
+      return true
+    })
     await expect(FailCmd.run([])).rejects.toThrow(/auth login/i)
     spy.mockRestore()
+    process.stdout.isTTY = origIsTTY
     expect(() => JSON.parse(writes.join(''))).toThrow()
   })
+
+  it('keeps a genuine internal error at exit 70', async () => {
+    mockLoadConfig.mockReturnValue({ activeProfile: 'default' })
+    class BoomCmd extends BaseCommand {
+      static skipAuth = true
+      async run() {
+        throw new Error('unexpected boom')
+      }
+    }
+    const origIsTTY = process.stdout.isTTY
+    process.stdout.isTTY = true
+    const err = await BoomCmd.run([]).catch((e) => e)
+    process.stdout.isTTY = origIsTTY
+    expect(err.exitCode ?? err.oclif?.exit).toBe(70)
+  })
+
+  it.each(['yaml', 'csv'])(
+    'emits a JSON error envelope (not %s) for a non-table default_output',
+    async (fmt) => {
+      mockLoadConfig.mockReturnValue({
+        activeProfile: 'default',
+        default_output: fmt,
+      })
+      class FailCmd extends BaseCommand {
+        static skipAuth = true
+        async run() {
+          throw new AuthRequiredError()
+        }
+      }
+      const writes = []
+      const spy = vi.spyOn(process.stderr, 'write').mockImplementation((c) => {
+        writes.push(String(c))
+        return true
+      })
+      await expect(FailCmd.run([])).rejects.toThrow()
+      spy.mockRestore()
+      const payload = JSON.parse(writes.join(''))
+      expect(payload.error).toBe('AuthRequiredError')
+      expect(payload.exitCode).toBe(77)
+    },
+  )
+
+  it('storedDefaultOutput swallows a throwing config read (error reporting must not crash)', () => {
+    mockLoadConfig.mockImplementation(() => {
+      throw new Error('corrupt config')
+    })
+    // Called by handleError while reporting another failure — must degrade to
+    // undefined (→ TTY/pipe fallback), never throw a secondary error.
+    const result = BaseCommand.prototype.storedDefaultOutput.call({ flags: {} })
+    expect(result).toBeUndefined()
+  })
 })