fix: message send and navigator message send fail at parse with Invalid argument spec

package.json

@@ -29,6 +29,7 @@
     "lint": "eslint src --fix",
     "format": "prettier --write --log-level warn \"src/**/*.ts\"",
     "manifest": "oclif manifest",
+    "test": "npm run build && node --test test/*.test.js",
     "typecheck": "tsc --noEmit",
     "prepublishOnly": "npm run clean && npm run build",
     "prepare": "husky",

src/commands/message/send.ts

@@ -1,6 +1,7 @@
 import { Args, Flags } from '@oclif/core';
 
 import { BaseCommand } from '@base-command';
+import { resolveMessagePositionals } from '@core/args/resolve-message-positionals';
 import { EXIT_CODE } from '@core/errors/exit-codes';
 import { formatVoidOutput } from '@core/output/formatter';
 import { runVoidWorkflow } from '@core/workflow/workflow-runner';
@@ -15,7 +16,7 @@ export default class MessageSend extends BaseCommand {
     }),
     text: Args.string({
       description: 'Message text (up to 1900 characters)',
-      required: true,
+      required: false,
     }),
   };
 
@@ -38,19 +39,29 @@ export default class MessageSend extends BaseCommand {
 
   public async run(): Promise<void> {
     const { args, flags } = await this.parse(MessageSend);
+    const { personUrl, text } = resolveMessagePositionals(args, flags['thread-id']);
 
-    if (!args['person-url'] && !flags['thread-id']) {
+    if (!personUrl && !flags['thread-id']) {
       this.error('Provide either a person-url argument or the --thread-id flag.', {
         exit: EXIT_CODE.VALIDATION,
       });
     }
 
+    if (!text) {
+      this.error(
+        'Provide the message text: message send <person-url> "<text>", or message send --thread-id <id> "<text>".',
+        {
+          exit: EXIT_CODE.VALIDATION,
+        },
+      );
+    }
+
     const client = await this.buildAuthenticatedClient();
 
     const params: Record<string, unknown> = {
-      personUrl: args['person-url'],
+      personUrl,
       threadId: flags['thread-id'],
-      text: args.text,
+      text,
     };
 
     if (flags.manage) {

src/commands/navigator/message/send.ts

@@ -1,6 +1,7 @@
 import { Args, Flags } from '@oclif/core';
 
 import { BaseCommand } from '@base-command';
+import { resolveMessagePositionals } from '@core/args/resolve-message-positionals';
 import { EXIT_CODE } from '@core/errors/exit-codes';
 import { formatVoidOutput } from '@core/output/formatter';
 import { runVoidWorkflow } from '@core/workflow/workflow-runner';
@@ -15,7 +16,7 @@ export default class NavigatorMessageSend extends BaseCommand {
     }),
     text: Args.string({
       description: 'Message text (up to 1900 characters)',
-      required: true,
+      required: false,
     }),
   };
 
@@ -36,8 +37,9 @@ export default class NavigatorMessageSend extends BaseCommand {
 
   public async run(): Promise<void> {
     const { args, flags } = await this.parse(NavigatorMessageSend);
+    const { personUrl, text } = resolveMessagePositionals(args, flags['thread-id']);
 
-    if (!args['person-url'] && !flags['thread-id']) {
+    if (!personUrl && !flags['thread-id']) {
       this.error('Provide either a person-url argument or the --thread-id flag.', {
         exit: EXIT_CODE.VALIDATION,
       });
@@ -48,15 +50,24 @@ export default class NavigatorMessageSend extends BaseCommand {
       });
     }
 
+    if (!text) {
+      this.error(
+        'Provide the message text: navigator message send <person-url> "<text>" --subject "<subject>", or navigator message send --thread-id <id> "<text>".',
+        {
+          exit: EXIT_CODE.VALIDATION,
+        },
+      );
+    }
+
     const client = await this.buildAuthenticatedClient();
 
     try {
       const result = await runVoidWorkflow(
         client.nvSendMessage,
         {
-          personUrl: args['person-url'],
+          personUrl,
           threadId: flags['thread-id'],
-          text: args.text,
+          text,
           subject: flags.subject,
         },
         {

src/core/args/resolve-message-positionals.ts

@@ -0,0 +1,34 @@
+interface TMessageArgs {
+  'person-url'?: string;
+  text?: string;
+}
+
+interface TMessagePositionals {
+  personUrl?: string;
+  text?: string;
+}
+
+/**
+ * oclif requires every optional arg to be declared after the required ones, so `text` cannot be
+ * declared as required while `person-url` stays optional for the --thread-id form. Both are declared
+ * optional and bound here instead: with --thread-id and a single positional, that positional is the
+ * message text, not a recipient.
+ */
+export function resolveMessagePositionals(
+  args: TMessageArgs,
+  threadId?: string,
+): TMessagePositionals {
+  const personUrl = args['person-url'];
+
+  if (threadId && args.text === undefined) {
+    return {
+      personUrl: undefined,
+      text: personUrl,
+    };
+  }
+
+  return {
+    personUrl,
+    text: args.text,
+  };
+}

test/message-send-args.test.js

@@ -0,0 +1,124 @@
+const assert = require('node:assert/strict');
+const { execFileSync } = require('node:child_process');
+const fs = require('node:fs');
+const path = require('node:path');
+const { describe, it, before } = require('node:test');
+
+const RUN = path.join(__dirname, '..', 'bin', 'run.js');
+const EMPTY_CONFIG_HOME = path.join(__dirname, '.tmp-config');
+
+// Port 1 is never bound, so a request can only fail to connect. The commands under test must stop at
+// the auth check long before this matters; it is here so a regression can never reach LinkedIn.
+const UNREACHABLE_API = 'http://127.0.0.1:1';
+
+const EXIT_AUTH = 2;
+const EXIT_VALIDATION = 5;
+
+function runCli(args) {
+  try {
+    const stdout = execFileSync(process.execPath, [RUN, ...args], {
+      encoding: 'utf-8',
+      stdio: ['ignore', 'pipe', 'pipe'],
+      timeout: 30_000,
+      env: {
+        ...process.env,
+        XDG_CONFIG_HOME: EMPTY_CONFIG_HOME,
+        LINKED_API_BASE_URL: UNREACHABLE_API,
+        NO_COLOR: '1',
+      },
+    });
+
+    return { exitCode: 0, stdout, stderr: '' };
+  } catch (error) {
+    return {
+      exitCode: error.status,
+      stdout: error.stdout ?? '',
+      stderr: error.stderr ?? '',
+    };
+  }
+}
+
+function assertReachedAuthCheck(result) {
+  assert.doesNotMatch(result.stderr, /Invalid argument spec/);
+  assert.match(result.stderr, /Authentication required/);
+  assert.equal(result.exitCode, EXIT_AUTH);
+}
+
+describe('message send argument spec', () => {
+  before(() => {
+    fs.rmSync(EMPTY_CONFIG_HOME, { recursive: true, force: true });
+  });
+
+  it('accepts the documented <person-url> <text> form', () => {
+    const result = runCli(['message', 'send', 'https://www.linkedin.com/in/john-doe', 'Hello John!']);
+
+    assertReachedAuthCheck(result);
+  });
+
+  it('binds the single positional to the text when --thread-id is given', () => {
+    const result = runCli(['message', 'send', '--thread-id', '2-abc123', 'Sounds good, talk soon!']);
+
+    assertReachedAuthCheck(result);
+  });
+
+  it('rejects a call with no recipient and no thread id', () => {
+    const result = runCli(['message', 'send']);
+
+    assert.equal(result.exitCode, EXIT_VALIDATION);
+    assert.match(result.stderr, /person-url|thread-id/);
+  });
+
+  it('rejects a recipient with no message text', () => {
+    const result = runCli(['message', 'send', 'https://www.linkedin.com/in/john-doe']);
+
+    assert.equal(result.exitCode, EXIT_VALIDATION);
+    assert.match(result.stderr, /message text/);
+  });
+});
+
+describe('navigator message send argument spec', () => {
+  before(() => {
+    fs.rmSync(EMPTY_CONFIG_HOME, { recursive: true, force: true });
+  });
+
+  it('accepts the documented <person-url> <text> form', () => {
+    const result = runCli([
+      'navigator',
+      'message',
+      'send',
+      'https://www.linkedin.com/in/john-doe',
+      'Hello!',
+      '--subject',
+      'Partnership',
+    ]);
+
+    assertReachedAuthCheck(result);
+  });
+
+  it('binds the single positional to the text when --thread-id is given', () => {
+    const result = runCli([
+      'navigator',
+      'message',
+      'send',
+      '--thread-id',
+      '2-abc123',
+      'Sounds good, talk soon!',
+    ]);
+
+    assertReachedAuthCheck(result);
+  });
+
+  it('rejects a recipient with no message text', () => {
+    const result = runCli([
+      'navigator',
+      'message',
+      'send',
+      'https://www.linkedin.com/in/john-doe',
+      '--subject',
+      'Partnership',
+    ]);
+
+    assert.equal(result.exitCode, EXIT_VALIDATION);
+    assert.match(result.stderr, /message text/);
+  });
+});