Repository navigation
fix(cli): refuse missing validator flag values by name - #412
Merged
Merged
Conversation
Fixes #288 what changed: path-authority and researcher-release now check value tokens before their handwritten parsers consume them. Missing, empty or next-flag tokens produce a named usage error on stderr with exit 2, before path resolution or input reading. Added a regression table in each CLI's existing validator test file for --file, --schema and --root at the end of the command or before --json. why: A bare --file reached path.resolve(undefined), surfacing a Node internal message or stack trace and, for researcher-release, exit 1 instead of a usage refusal (#288). Apply the existing usage-error boundary; no alternative design was set aside. what it touched: the two validator CLI parsers, path-authority/release-validator tests, their architecture descriptions, changelog and README test inventory. Repeated --file values remain supported; validation statuses and their exit-code mapping are unchanged. what you verified: verified - red-first 38 passed and two expected failures, reproducing the path-authority JSON-READ/paths[0] message and researcher-release TypeError/exit 1. Then path-authority, release-validator and support-policy passed 40 tests with zero failures in 210.9 seconds, including named exit-2 refusals and normal CLI conformance/validation. Architecture-map passed 21 tests; handoff OK (33 ledger entries, 27 evidence rows), preflight PASS (28 passing, one warning, no blockers), doctor READY collector/firecrawl-cli. After refresh onto merged main 82a02cb, the same 40 focused tests passed with zero failures and zero unsupported cases in 14.5 seconds. The patch was then advanced to main 194bcfb after PR411 landed; staged code/test blobs match the focused-tested source, while shared documentation includes that fix and the 1,838-test inventory. The normal full commit gate runs before acceptance; GitHub CI must verify the published head before merge. what you got wrong and fixed: nothing to report.
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
1 participant
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.
Bare or empty value flags in path-authority and researcher-release previously reached Node path resolution and leaked an internal message or TypeError stack. Both CLIs now refuse the value by name with usage and exit 2 before reading input.
Fixes #288
what changed: path-authority and researcher-release now check value tokens before their handwritten parsers consume them. Missing, empty or next-flag tokens produce a named usage error on stderr with exit 2, before path resolution or input reading. Added a regression table in each CLI's existing validator test file for --file, --schema and --root at the end of the command or before --json.
why: A bare --file reached path.resolve(undefined), surfacing a Node internal message or stack trace and, for researcher-release, exit 1 instead of a usage refusal (#288). Apply the existing usage-error boundary; no alternative design was set aside.
what it touched: the two validator CLI parsers, path-authority/release-validator tests, their architecture descriptions, changelog and README test inventory. Repeated --file values remain supported; validation statuses and their exit-code mapping are unchanged.
what you verified: verified - red-first 38 passed and two expected failures, reproducing the path-authority JSON-READ/paths[0] message and researcher-release TypeError/exit 1. Then path-authority, release-validator and support-policy passed 40 tests with zero failures in 210.9 seconds, including named exit-2 refusals and normal CLI conformance/validation. Architecture-map passed 21 tests; handoff OK (33 ledger entries, 27 evidence rows), preflight PASS (28 passing, one warning, no blockers), doctor READY collector/firecrawl-cli. After refresh onto merged main 82a02cb, the same 40 focused tests passed with zero failures and zero unsupported cases in 14.5 seconds. The patch was then advanced to main 194bcfb after PR411 landed; staged code/test blobs match the focused-tested source, while shared documentation includes that fix and the 1,838-test inventory. The normal full-suite commit gate accepted 3770719 on Windows / Node 24.20.0, with its documented local unsupported-capability allowance. GitHub CI will verify the published head before merge.
what you got wrong and fixed: nothing to report.