From f7a7e7f674531dd47cef95a1143f9a02a983918c Mon Sep 17 00:00:00 2001 From: Vinicius Freitas Date: Mon, 27 Jul 2026 21:55:23 -0300 Subject: [PATCH] fix(message): make message send parse again Both `message send` and `navigator message send` failed at parse time with "Invalid argument spec" and never reached the API. oclif requires every optional argument to be declared after the required ones, and both commands declare the optional `person-url` before the required `text`. Declaring `text` as optional alone is not enough: with `--thread-id` and a single positional, oclif binds that positional to `person-url`, so the CLI would send an empty message. Both positionals are now declared optional and bound in `resolveMessagePositionals`, which maps a lone positional to the message text when `--thread-id` is present. A missing text is rejected with exit code 5 instead of silently sending nothing. Adds a test suite covering both commands and an `npm test` script using the Node built-in test runner, so no new dependency is introduced. --- package.json | 1 + src/commands/message/send.ts | 19 ++- src/commands/navigator/message/send.ts | 19 ++- src/core/args/resolve-message-positionals.ts | 34 +++++ test/message-send-args.test.js | 124 +++++++++++++++++++ 5 files changed, 189 insertions(+), 8 deletions(-) create mode 100644 src/core/args/resolve-message-positionals.ts create mode 100644 test/message-send-args.test.js diff --git a/package.json b/package.json index 07508d4..07b621d 100644 --- a/package.json +++ b/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", diff --git a/src/commands/message/send.ts b/src/commands/message/send.ts index 95b284f..dd7977d 100644 --- a/src/commands/message/send.ts +++ b/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 { 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 "", or message send --thread-id "".', + { + exit: EXIT_CODE.VALIDATION, + }, + ); + } + const client = await this.buildAuthenticatedClient(); const params: Record = { - personUrl: args['person-url'], + personUrl, threadId: flags['thread-id'], - text: args.text, + text, }; if (flags.manage) { diff --git a/src/commands/navigator/message/send.ts b/src/commands/navigator/message/send.ts index f01c289..9a933bf 100644 --- a/src/commands/navigator/message/send.ts +++ b/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 { 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 "" --subject "", or navigator message send --thread-id "".', + { + 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, }, { diff --git a/src/core/args/resolve-message-positionals.ts b/src/core/args/resolve-message-positionals.ts new file mode 100644 index 0000000..ae3ec35 --- /dev/null +++ b/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, + }; +} diff --git a/test/message-send-args.test.js b/test/message-send-args.test.js new file mode 100644 index 0000000..afef468 --- /dev/null +++ b/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 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 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/); + }); +});