fix: message send and navigator message send fail at parse with Invalid argument spec - #3
Open
viniciuscffreitas wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is broken
linkedin message sendandlinkedin navigator message senddo not run at all. Every invocation, including the exact examples in the README, stops at argument parsing:Reproduced on the published
@linkedapi/linkedin-cli@1.2.7and on a clean build ofmain. The command never reaches the API, so sending a message through the CLI is not possible today.Cause
oclif requires every optional argument to be declared after the required ones. Both commands declare the optional
person-urlbefore the requiredtext, so oclif rejects the spec before parsing anything. I scanned all 71 commands and only these two files have the pattern.Why relaxing
requiredis not enoughMaking
textoptional on its own creates a worse bug. With--thread-idand one positional, oclif binds that positional toperson-url, so the CLI would call the API with the message body in the recipient field and no text at all. Nothing would fail loudly.Both positionals are now declared optional and bound explicitly in a small helper,
resolveMessagePositionals. When--thread-idis present and only one positional was given, that positional is the message text. A missing text is now rejected with the validation exit code instead of sending an empty message.Verification
Captured the outgoing request body against a local mock of the API, so no LinkedIn traffic was involved:
The second and third lines are the point: the lone positional lands in
textand nopersonUrlis sent.Tests
Added
test/message-send-args.test.jswith 7 cases covering both commands: the documented two positional form, the--thread-idform, and the rejection paths. All 7 fail on currentmainand pass with this change. They run the built CLI as a child process with an empty config directory and an unreachable API base URL, so they stop at the authentication check and can never produce outbound traffic.The repo had no test script, so I added one using the Node built in test runner. No new dependency:
npm run typecheck,npm run lintandnpm run buildare all clean.Scope
Only this defect. I left the existing behavior where action level errors exit 0 with
{success: false}untouched, sinceCLAUDE.mddocuments that as intended.