fix: prevent registerSubcommand from injecting false positional args#111
Merged
fix: prevent registerSubcommand from injecting false positional args#111
Conversation
The enriched usage string (e.g., `create --slug <string> --name <string>`) was passed directly as the yargs command string. Yargs interprets `<...>` tokens in command strings as required positional arguments, causing commands like `permission create` and `role create` to demand phantom positional args alongside the actual named options. Move the enriched string to `.usage()` (display-only for help text) and pass only the original command string to `yargs.command()`. Fixes: commands with demandOption flags (permission create, role create, invitation create, seed, etc.) rejecting valid `--flag value` invocations with "Not enough non-option arguments".
Exercises yargs arg parsing end-to-end for all 16 commands that use registerSubcommand with demandOption named flags. Each test verifies that standard --flag syntax is accepted without "Not enough non-option arguments" errors. The existing smoke test bypasses yargs entirely (calls handlers directly), so it could not catch the parsing bug. These tests fill that gap.
Move the required-flag annotation from the yargs command string to the description. This fixes two issues with the previous approach: - P2: `$0` in `.usage()` resolved to the root script name, so nested commands like `workos role create --help` showed `workos create ...` instead of `workos role create ...` - P3: parent help lost enrichment entirely since the command string was reverted to plain usage Now parent help shows e.g.: create Create a permission (requires --slug, --name) Positional args already visible in the command string (e.g., <slug>) are excluded from the description enrichment to avoid redundancy.
Commands like `role delete` and `role remove-permission` already include `(requires --org)` in their description. The enrichment helper now filters out flags that the description already references, preventing duplicated text like `(requires --org) (requires --org)`.
Convert 15 copy-paste test cases into data-driven it.each blocks and remove unused YargsOptions fields (string, number, boolean arrays).
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.
Summary
Fixes a bug where commands using named
--optionswithdemandOption: true(e.g.,permission create,role create,invitation create) rejected valid invocations with:Root cause
registerSubcommand()probes the builder to discover required options, then enriches the usage string:This enriched string was passed directly as the command argument to
yargs.command(). Yargs interprets<...>tokens in the command string as required positional arguments, so it parsed this as:create--slug<string>--name<string>This created 4 phantom positional requirements on top of the actual
--slugand--nameoptions, making the command impossible to use with standard flag syntax.Workaround users discovered
Fix
create) as the yargs command — preserving any real positionals like<slug>on commands that use them (e.g.,update <slug>,delete <slug>).usage(), which is display-only and only affects--helpoutputBefore (broken)
After (fixed)
Affected commands
Any subcommand registered via
registerSubcommandwithdemandOptionoptions:permission create/role create/invitation createseed(multiple required options)setup-org,onboard-user(mixed positional + required options)organization,user,membership, etc.Commands using only positional args (e.g.,
permission delete <slug>) or no required options (e.g.,permission list) were unaffected.Verified
workos permission create --slug "test-perm" --name "Test Permission"→Not enough non-option arguments: got 0, need at least 4./dist/bin.js permission create --slug "test-perm" --name "Test Permission"→ successfully creates permissionTest plan
pnpm buildpassespnpm testpasses (all 1509 tests across 117 files)pnpm typecheckpassespermission create --slug --nameworks on patched build--helpoutput still shows required options in usage lineupdate <slug>,delete <slug>) still work