fix(cli): report unknown-flag errors + cover nested subcommands (HF#2033) [P2]#2072
Merged
Conversation
…033) Two flag-hygiene gaps behind the assertKnownFlags arc: 1. Telemetry loss: assertKnownFlags ran BEFORE the try/catch in the command wrapper, so an unknown-flag throw skipped reportCommandFailure entirely — zero signal on how often users hit bad flags. Moved the assertion inside the try so it reports like any other failure. 2. Nested-subcommand scope: cli.ts wraps only the top-level command loaders, so command groups' leaves (cloud/*, auth/*, figma/*, lambda/*, capture/*, skills) were never wrapped — citty dispatches to the leaf, whose run had no assertion and no failure reporting. So `hyperframes cloud render --badflag` silently ignored the flag. trackCommandFailures now recurses through cmd.subCommands (normalizing citty's Resolvable entries to loaders) and wraps every leaf. Identity is preserved for bare no-run/no-subcommand defs. Verified: `auth status --badflag` now errors "Unknown flag: --badflag" (previously silent); `auth --help` still dispatches; top-level `lint --badflag` still rejected. Tests: unknown-flag rejection is reported, and a nested subcommand's failure reaches onFailure.
CI Typecheck (tsc, unlike the local tsup build) flagged the nested-subcommand test: indexing `subCommands["render"]` yields `T | undefined` under noUncheckedIndexedAccess, so invoking it tripped TS2722/TS18048. Guard the loader before calling it.
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.
Addresses the two OPEN items from Rames' HF#2033 review (the load-bearing hardening behind the flag-hygiene arc, e.g. #2067).
1. Unknown-flag errors weren't reported to telemetry
assertKnownFlagsran before thetry/catchin the command wrapper, so an unknown-flag throw bypassedreportCommandFailure— no signal on how often users hit bad flags. Moved the assertion inside the try so it's reported like any other failure.2. Nested command groups weren't covered
cli.tswraps only the top-level command loaders. citty dispatcheshyperframes cloud render --badflagto therenderleaf, whoserunwas never wrapped — so nested groups (cloud/*,auth/*,figma/*,lambda/*,capture/*,skills) silently ignored unknown flags and reported nothing.trackCommandFailuresnow recurses throughcmd.subCommands(normalizing citty'sResolvableentries to loaders) and wraps every leaf. Object identity is preserved for bare no-run/no-subcommand defs.Verified E2E
auth status --badflag→Error: Unknown flag: --badflag(previously silently ignored)auth --helpstill dispatches correctly (no regression)lint --badflagstill rejectedTests
+2: an unknown-flag rejection is routed through
onFailure; a nested subcommand's failure reachesonFailure. 15 tracker+reject tests pass.