fix(args): warn when positionals overwrite an explicit --_ flag#39
Conversation
The reserved `_` key silently clobbered a user-declared `--_` flag in validateInput. Emit a loud stderr warning when both an explicit `--_` flag and positional args are present, and document `_` as reserved in ParsedArgs and SubcommandSpec.input. Behavior is unchanged — positionals still win; stdout stays reserved for the JSON envelope. Closes #19 Co-authored-by: Juan Ignacio Gipponi <beogip@users.noreply.github.com>
Greptile SummaryThis PR fixes the silent
Confidence Score: 5/5Safe to merge — the change is a pure additive warning on stderr with no mutation of the existing overwrite logic. The implementation is a small, focused addition of a single Object.hasOwn guard and a process.stderr.write call inside the existing if (parsed.positional.length > 0) branch. Existing behavior (positionals win, stdout carries only the JSON envelope) is entirely unchanged. All three edge cases are covered by unit tests with proper spy/restore, and end-to-end subprocess tests confirm the warning reaches stderr without corrupting stdout. No files require special attention.
|
| Filename | Overview |
|---|---|
| src/args.ts | Adds a precise Object.hasOwn guard and process.stderr.write warning before the existing toValidate._ = parsed.positional assignment; logic is correct and the overwrite behavior is unchanged. |
| src/types.ts | JSDoc-only changes: accurately documents _ as reserved across ParsedArgs.flags, the ParsedArgs interface, and SubcommandSpec.input. |
| tests/args.test.ts | Adds three unit tests using spyOn(process.stderr, 'write') covering all three edge cases (collision, positionals-only, flag-only), all with proper mockRestore in finally blocks. |
| tests/cli.test.ts | Adds two black-box subprocess tests asserting the warning reaches stderr and positionals win on stdout; correctly asserts exit code 0 in both cases. |
| tests/fixtures/dummy-tool.ts | Adds the echoArgs fixture subcommand with _: z.array(z.string()).optional() schema and a _ ?? [] handler, correctly supporting the CLI collision tests. |
| docs/specs/issue-19-reserved-key-positionals-silently-overwrite.md | Implementation plan artifact; matches the delivered changes and acceptance criteria exactly. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["CLI: argv"] --> B["parseArgs(argv)"]
B --> C{"positional.length > 0?"}
C -- No --> E["safeParse(flags only)"]
C -- Yes --> D{"Object.hasOwn(flags, '_')?"}
D -- No --> F["toValidate._ = positional"]
D -- Yes --> G["process.stderr.write(warning)"]
G --> F
F --> E
E --> H["Zod safeParse result"]
Reviews (1): Last reviewed commit: "fix(args): warn when positionals overwri..." | Re-trigger Greptile
|
🎉 This PR is included in version 0.1.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Fixes the footgun reported in #19: in
validateInput(src/args.ts) positional args are attached under the reserved_key, silently overwriting any user-declared--_flag.This PR addresses both remedies suggested in the issue:
--_flag and positional args are both present, a warning is written to stderr (stdout stays reserved for the JSON envelope, per the always-exit-0 / loud-failure contract)._as reserved — in the JSDoc forvalidateInput,ParsedArgs, andSubcommandSpec.input.Behavior is unchanged and backward compatible — positionals still win the collision; only a warning is added.
Changes
src/args.ts— emit stderr warning when both--_and positionals are present; document reserved_.src/types.ts— document_as reserved inParsedArgsandSubcommandSpec.input.tests/args.test.ts— unit tests (spy onprocess.stderr.write): warns only when both present, silent otherwise.tests/fixtures/dummy-tool.ts— newechoArgssubcommand declaring the reserved_key.tests/cli.test.ts— black-box test asserting the warning reaches stderr and positionals win on stdout.docs/specs/issue-19-...md— implementation plan artifact.Verification
bun test— 118 pass / 0 failbunx tsc --noEmit— cleanbunx biome check .— cleanCloses #19
Generated with Claude Code