fix(errors): duck-typing for cross-package CliError in toEnvelope (#2388)

* fix(errors): duck-typing for cross-package CliError in toEnvelope

Plugins resolve their own copy of @jackwener/opencli (own node_modules), so
 fails across package copies and every plugin error
degrades to code: UNKNOWN with the hint lost. Switch to shape-based detection
(code + message strings, optional hint) — real CliError instances behave
identically, plain Errors still map to UNKNOWN.

Also serializes error-like plain objects carrying code/message. Adds tests
for the cross-package shape, plain-object passthrough, and UNKNOWN fallback.

* fix(errors): require exitCode when duck-typing CliError

The shape check accepted any object with string code+message, which also
matches Node system errors (ENOENT, ECONNREFUSED, EACCES) and library
errors carrying a string code. Those would surface their errno as the
envelope code, widening the machine-readable contract callers switch on:

  before this commit: ENOENT -> code "ENOENT"
  intended/base:      ENOENT -> code "UNKNOWN"

CliError's constructor always assigns exitCode (defaulting to
GENERIC_ERROR) while Node system errors never do, so requiring a numeric
exitCode separates genuine cross-package CliError copies from foreign
errors without adding any new concept.

Tests: make the cross-package mock faithful by giving it exitCode, pin
the bare {code,message} object as UNKNOWN, and add an ENOENT regression.

---------

Co-authored-by: jackwener <jakevingoo@gmail.com>

src/errors.test.ts

@@ -119,6 +119,48 @@ describe('toEnvelope', () => {
     expect(envelope.error.message).toBe('string error');
   });
 
+
+  it('passes through cross-package CliError copies (duck-typed shape)', () => {
+    // Simulates a CliError thrown by a plugin that resolves its own copy of
+    // @jackwener/opencli — different class identity, same shape.
+    class ForeignCliError extends Error {
+      code = 'INVALID_ARGS';
+      hint: string | undefined;
+      // A real CliError always assigns exitCode in its constructor, so a
+      // faithful cross-package copy carries it too.
+      exitCode = 2;
+      constructor(message: string, hint?: string) {
+        super(message);
+        this.name = 'CliError';
+        this.hint = hint;
+      }
+    }
+    const envelope = toEnvelope(new ForeignCliError('bad file', 'pass a real path'));
+    expect(envelope.error.code).toBe('INVALID_ARGS');
+    expect(envelope.error.help).toBe('pass a real path');
+    expect(envelope.error.message).toBe('bad file');
+  });
+
+  it('does not treat a bare {code,message} object as a CliError', () => {
+    // No exitCode => not CliError-shaped. Accepting these would let any
+    // foreign string `code` into the envelope contract.
+    const envelope = toEnvelope({ code: 'FORBIDDEN', message: 'scope violation' });
+    expect(envelope.error.code).toBe('UNKNOWN');
+  });
+
+  it('keeps Node system errors as UNKNOWN instead of surfacing their errno', () => {
+    // fs/net errors have a string `code` and `message` but no exitCode.
+    // Reporting `ENOENT` as the envelope code would widen the machine-readable
+    // contract that callers switch on.
+    const enoent = Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' });
+    expect(toEnvelope(enoent).error.code).toBe('UNKNOWN');
+  });
+
+  it('keeps UNKNOWN for Errors without a code', () => {
+    const envelope = toEnvelope(new Error('random failure'));
+    expect(envelope.error.code).toBe('UNKNOWN');
+  });
+
   it('serializes deep cause chains without stack overflow', () => {
     // Build a 20-level deep cause chain — should truncate at depth 10
     let deepErr: Error = new Error('root');

src/errors.ts

@@ -257,14 +257,31 @@ export function toEnvelope(err: unknown): ErrorEnvelope {
     receiptPath: traceReceipt.receiptPath,
     status: traceReceipt.status,
   } : undefined;
-  if (err instanceof CliError) {
+  // Duck typing: accept own CliError instances AND cross-package copies that
+  // carry the same shape. `instanceof` fails when the throwing module resolves
+  // a different copy of @jackwener/opencli (e.g. a plugin with its own
+  // node_modules) — those errors used to degrade to UNKNOWN and lose `hint`.
+  //
+  // `exitCode` is the discriminator: CliError's constructor always assigns it
+  // (defaulting to GENERIC_ERROR), while Node system errors carry a string
+  // `code` (ENOENT, ECONNREFUSED, EACCES) and a string `message` but never an
+  // `exitCode`. Without this check those would be reported with their errno as
+  // the envelope `code`, silently widening the contract that callers switch on.
+  const isCliErrorLike =
+    err !== null &&
+    typeof err === 'object' &&
+    typeof (err as any).code === 'string' &&
+    typeof (err as any).message === 'string' &&
+    typeof (err as any).exitCode === 'number';
+  if (err instanceof CliError || isCliErrorLike) {
+    const e = err as any;
     return {
       ok: false,
       error: {
-        code: err.code,
-        message: err.message,
-        ...(err.hint ? { help: err.hint } : {}),
-        exitCode: err.exitCode,
+        code: e.code,
+        message: e.message,
+        ...(typeof e.hint === 'string' && e.hint ? { help: e.hint } : {}),
+        exitCode: e.exitCode ?? EXIT_CODES.GENERIC_ERROR,
         ...(cause ? { cause } : {}),
       },
       ...(trace ? { trace } : {}),