Repository navigation
fix(cli): exit on an unknown subcommand instead of blocking on stdin - #1444
Merged
Merged
Conversation
A typo'd subcommand fell through the dispatcher to the plan-hook path, which reads the hook payload from stdin. From a terminal that stdin never closes, so `plannotator annotatte README.md` hung with no output instead of reporting the typo. Reject an unrecognized first token before the dispatcher runs: print the offending word, the nearest documented command, and a pointer to --help, then exit 1. Flags and the no-argument hook invocation are left alone so the real hook path is untouched.
Owner
|
Merged. This was an exemplary report and fix: the diagnosis named the exact fallthrough (the plan-mode hook's stdin read), the guard exempts precisely the paths that must keep working (no-arg hook invocation, flags, per-subcommand parsing), and the registry-equality test means the dispatcher can never drift past it. Independent review confirmed all sixteen dispatched subcommands pass the guard and the end-to-end test genuinely reproduces the hang. Thank you — this will ship in the next release. AI-assisted (Claude) under maintainer direction. |
backnotprop
added a commit
that referenced
this pull request
Sep 1, 2026
…s on CI The e2e case from #1444 spawns the real CLI, whose module load imports ../dist/index.html and ../dist/review.html. Dev machines always have those built, but CI's test job does not build the apps, so the spawn died at import time with 'Cannot find module' before the guard ran and the assertion on the unknown-command message failed, breaking main. The test now drops empty placeholders for any missing artifact before spawning and removes exactly what it created afterwards; a real dist is never touched (verified with sentinel content). The guard under test exits before either page is served, so placeholder content is irrelevant.
1 task
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.
A typo'd subcommand produced no output and no exit.
plannotator annotatte README.mdsimply hung.The cause is the dispatcher's shape.
apps/hook/server/index.tsroutes on a chain ofargs[0] === "…"tests, and the chain's finalelseis plan review mode, which opens withawait Bun.stdin.text()to read thePermissionRequesthook event. That fallthrough is correct for the hook, which is spawned with a payload on stdin and a closed pipe. From a terminal there is no payload and stdin never closes, so an unrecognized word inherits the hook path and waits forever. Nothing distinguished "the hook invoked us" from "the user misspelled a command".The guard
One check before the dispatcher runs, at
apps/hook/server/index.ts:388, immediately after the--versionand--helpbranches and before every subcommand test:On stderr, exit 1.
What it refuses to touch. The guard fires only on a first token that is a bare word not in the registry. An empty
argsreturns null, so the no-argument hook invocation still reaches stdin andisInteractiveNoArgInvocationstill gets its TTY clarification. A leading-returns null, so--help,-vandplannotator --browser <name>are untouched. Onlyargs[0]is examined, so a bad flag on a good subcommand (review --nonsense) stays with the subcommand's own parser, which already reports it properly.The registry cannot drift.
KNOWN_SUBCOMMANDSisSUBCOMMAND_HELPplusSUBCOMMAND_HELP_ALIASESfromcli.ts, plus five internal commands that are dispatched but deliberately undocumented (install-runtime,opencode-plan,opencode-review,opencode-annotate-last,copilot-plan). A registry that falls behind the dispatcher would reject a real command, which is worse than the hang it replaces, so the test scrapes everyargs[0] === "…"literal out ofindex.tsand asserts set equality withKNOWN_SUBCOMMANDS. Adding a subcommand without registering it fails the suite.Suggestions come from the documented commands only, never the internal five. Prefix match first for tokens of three characters or more (
guid→guide,annot→annotate), then Levenshtein within a length-scaled tolerance of 1/2/3. A candidate whose length differs by more than the tolerance is skipped before the distance is computed, which is what keeps a 10,000-character token cheap. Below three characters nothing is suggested —ahas too many neighbours to guess at — and the message then carries only the--helpline.Tests
apps/hook/server/unknown-subcommand.test.ts, six cases: the registry/dispatcher equality above, the flag and no-argument exemptions, the reported token, the suggestion cascade including the two null cases, the message content, and one end-to-end spawn with an open stdin pipe asserting exit 1 and the error text. That last one is the regression itself — with the guard removed it hangs until the 15s kill.Footprint
apps/hook/server/unknown-subcommand.ts(new)apps/hook/server/unknown-subcommand.test.ts(new)apps/hook/server/index.tsVerification
bun test→ 4120 pass, 787 skip, 3 fail. The three failures (git-background.test.ts× 2,review-core.test.ts× 1) reproduce on a cleanmaincheckout with the branch stashed, so they are unrelated.bun run typecheckclean.