Skip to content

fix(cli): reject unknown top-level commands instead of silent ask fallthrough - #27

Merged
andrei-hasna merged 1 commit into
mainfrom
fix/audit-knowledge-unknown-command-fallthrough
Jul 24, 2026
Merged

fix(cli): reject unknown top-level commands instead of silent ask fallthrough#27
andrei-hasna merged 1 commit into
mainfrom
fix/audit-knowledge-unknown-command-fallthrough

Conversation

@andrei-hasna

Copy link
Copy Markdown
Contributor

Bug

knowledge boguscmd (and any unknown/typo top-level command such as knowledge lst) silently routed to the ask/build search flow: it printed Prepared citation context draft (or a JSON context pack under --json) and exited 0, returning false success to scripts and users. Severity: high (slug: unknown-command-fallthrough).

Root cause

In src/cli.ts, when the CLI is invoked as the installed knowledge bin, the dispatcher remapped any non-command positional to ask:

if (invokedAsKnowledge() && command && !COMMANDS.includes(command)) {
  command = 'ask';
  commandArgOffset = 0;
}

This intentionally powers the documented natural-language shorthand knowledge <prompt>, but it also swallowed single-token typos of real commands. The existing unknown-command handler (with a levenshtein "Did you mean" suggestion and non-zero exit) was therefore unreachable for the real bin — it only fired for non-knowledge invocations (e.g. tests), which is why the pre-existing unknown command includes suggestion test passed while the shipped bin misbehaved.

Fix

Gate the shorthand so it only applies to genuine multi-word natural-language prompts:

if (invokedAsKnowledge() && command && !COMMANDS.includes(command) && positional.length > 1) {

A single unknown token (boguscmd, lst) is now treated as a mistyped command and falls through to the existing unknown-command handler → exits 1 with Unknown command: <x>. Did you mean '<y>'? Run 'knowledge --help' .... Multi-word prompts (knowledge how do I cite sources) still route to ask, preserving the documented knowledge <prompt> shorthand and the existing runKnowledgeBin(['Generate','fake','answer', ...]) contract test.

Tests

Added to tests/cli.test.ts (both use runKnowledgeBin, which invokes a wrapper literally named knowledge, so invokedAsKnowledge() is true — the condition under which the bug reproduces):

  • knowledge bin rejects unknown single-token command instead of running ask — asserts boguscmd and lst exit 1, do not print Prepared citation context draft, and emit Unknown command (plus the Did you mean 'list' suggestion for lst). Fails before the fix, passes after.
  • knowledge bin keeps multi-word natural-language ask shorthand — asserts knowledge how do I cite sources still exits 0 and runs the ask flow with the prompt preserved.

Repro (after fix)

$ knowledge boguscmd ; echo $?
Error: Unknown command: boguscmd. Run 'knowledge --help' for available commands.
1
$ knowledge lst
Error: Unknown command: lst. Did you mean 'list'? Run 'knowledge --help' for available commands.
$ knowledge how do I cite sources --json   # NL shorthand still works, exit 0

Verification

  • New tests pass; tests/cli.test.ts shows the same pre-existing failures on this branch as on origin/main (all timeout/environment-related — identical set), so no regressions introduced.
  • No version bump, no publish, artifacts (bin/, dist/) intentionally left to be regenerated at build/publish time.

🤖 Generated with Claude Code

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Adversarial review: REQUEST_CHANGES

The fix correctly stops the reported bug: knowledge boguscmd and knowledge lst now exit 1 with Unknown command: ... (verified before/after against the parent commit). The regression tests pass, the diff is minimal (2 files), and there are no secrets.

However, the positional.length > 1 heuristic conflates number of shell tokens with typo-vs-prompt, and this breaks a documented interface.

Confirmed regression: the quoted natural-language shorthand now fails

README.md documents (line 227, and generically line 683 knowledge <prompt>):

knowledge "How do we cite handbook policy?" --scope project --json

That whole quoted string is a single positional argument, so positional.length === 1 and the new guard skips the ask-remap — it falls through to the unknown-command handler.

Reproduced with a real knowledge-named bin wrapper:

  • Parent commit: knowledge "How do we cite handbook policy?" --scope project --json → exit 0 (ask context pack).
  • This PR: same command → Error: Unknown command: How do we cite handbook policy? exit 1.

So a legitimate, documented invocation now returns false failure. The new "keeps multi-word natural-language ask shorthand" test only exercises the unquoted 5-token form (how do I cite sources), so it does not catch this.

Suggested fix

Discriminate on whitespace instead of token count — a mistyped command has no spaces, a prompt does:

const looksLikePrompt = positional.length > 1 || /\s/.test(positional[0] ?? '');
if (invokedAsKnowledge() && command && !COMMANDS.includes(command) && looksLikePrompt) {
  command = 'ask';
  commandArgOffset = 0;
}

This still rejects knowledge boguscmd / knowledge lst while preserving knowledge "<quoted prompt>". Please also add a regression test for the quoted single-arg form. (If dropping the quoted shorthand is intentional, update README lines 227 and 683 accordingly — but that would be a doc-breaking change to flag separately.)

Test suite note

bun test tests/cli.test.ts has ~8 pre-existing failures on both the parent commit and this branch (sync/inventory/legacy/setup — flaky 5s timeouts, environmental). None relate to command routing; not introduced by this PR.

…ilent ask fallthrough

When invoked as the `knowledge` bin, any non-command positional was remapped
to an implicit `ask`/`build` search prompt. A mistyped command like
`knowledge lst` or a bogus one like `knowledge boguscmd` therefore printed a
citation-context draft and exited 0, returning false success to scripts.

Only remap to `ask` when the input looks like a natural-language prompt:
either multiple positional words (`knowledge how do I cite sources`) or a
single quoted token that contains whitespace (`knowledge "How do we cite the
handbook?"`, the canonical documented form). A single bare token with no
whitespace now falls through to the existing unknown-command handler, which
exits non-zero with an 'Unknown command' message and a levenshtein suggestion.

Rebuilt on the reconciled npm-line main (0.2.86). Tests use an isolated HOME
and explicit timeouts; a regression test covers the quoted single-token prompt.
@andrei-hasna
andrei-hasna force-pushed the fix/audit-knowledge-unknown-command-fallthrough branch from 65496ae to 4cba943 Compare July 24, 2026 15:09
@andrei-hasna
andrei-hasna merged commit eea19c5 into main Jul 24, 2026
3 of 7 checks passed
@andrei-hasna
andrei-hasna deleted the fix/audit-knowledge-unknown-command-fallthrough branch July 24, 2026 15:09
andrei-hasna added a commit that referenced this pull request Jul 24, 2026
Bring in #22/#26/#27/#29/#3 (hosted-url scrub, --json error contract,
reject unknown top-level commands, FTS query parser, public-package
docs hardening). Renumber this change to 0.2.90 (main consumed 0.2.88
and 0.2.89); rebuild bin/ and dist/ from merged sources.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant