feat(cli): add shell tab-completion (bash, zsh, fish)#234
Open
MQ37 wants to merge 2 commits into
Open
Conversation
Member
|
Cool. Pls let's wait with this for the next release |
Collaborator
Author
@jancurn we need to first migrate jest to vitest for testing as jest does not support ESM modules and would require lot of hacking around - so I will first introduce migration PR to vitest and strack this one on top |
- Replace jest+ts-jest with vitest+@vitest/coverage-v8 - vitest.config.ts mirrors the previous Jest config (Node env, same test glob, v8 coverage at 70% thresholds) - Test API stays Jest-compatible via 'globals: true': describe/it/expect unchanged, jest.fn/jest.mock/jest.spyOn renamed to vi.* equivalents - Type annotations: jest.SpyInstance -> MockInstance, jest.Mock -> Mock, imported as types from 'vitest' in the four files that use them - Convert arrow-function mockImplementation factories to regular functions in 3 places (factory/transports/keychain tests) so 'new MockedClass()' works under vitest (arrow functions are not constructable) - Use vi.hoisted() in grep.test.ts where the chalk mock factory referenced a const declared after vi.mock() - Drop the chalk/ora transformIgnorePatterns workarounds; vitest runs ESM natively - Update test/README.md and the keychain.ts comment to drop Jest references Build, lint, and full unit suite (581 tests) pass locally. CI workflow unchanged (still 'pnpm run test:unit').
MQ37
force-pushed
the
feat/shell-completion
branch
from
3bda841 to
231881b
Compare
Two-piece pattern (kubectl/gh/cobra-style): - 'mcpc completion <bash|zsh|fish>' prints a thin shell script that registers a completion function for the user's shell. - 'mcpc __complete -- <words...> <partial>' is a hidden subcommand the script calls on every TAB. Returns candidates on stdout (one per line) plus an optional ':<bitflag>' directive line. - 'mcpc completion install [shell]' auto-detects the user's shell and writes the script to the standard path. Completes top-level commands, session subcommands, @session names from ~/.mcpc/sessions.json, auth-server hosts, log levels, all known flags (introspected from Commander's tree at runtime so there is no static drift), and tool / resource URI / prompt names for connected sessions. Tool/resource/prompt names are mirrored to ~/.mcpc/completion/<session>.json whenever the user runs 'tools-list' / 'resources-list' / 'prompts-list', so TAB completion is fast and never triggers network calls or OAuth flows. The cache file is removed alongside the session by 'mcpc clean sessions'. createTopLevelProgram, createSessionProgram, and registerSessionCommands are now exported from src/cli/index.ts so commands/completion.ts can walk the Commander tree to extract option flags. index.ts no longer self-runs on import; bin/mcpc calls mod.run() explicitly, and a small direct-run guard supports 'tsx src/cli/index.ts' invocation for the README-help test. Reading package.json via JSON import attribute replaces the old createRequire(import.meta.url) pattern. Stacks on top of #236 (Jest -> Vitest migration); the vitest move removes the need for chalk mocking and the src/cli/main.ts split that earlier revisions of this branch carried.
MQ37
force-pushed
the
feat/shell-completion
branch
from
231881b to
5f6fe95
Compare
jancurn
pushed a commit
that referenced
this pull request
May 18, 2026
> 📚 **PR #234 stacks on top of this one** (`feat(cli): add shell tab-completion`). Once this lands, the shell-completion branch drops its chalk mock and `src/cli/main.ts` workaround. ## Context Jest's ESM story is rough on this repo: pure-ESM deps like `chalk` and `ora` need `transformIgnorePatterns` workarounds, `import.meta` in `src/cli/index.ts` trips the CJS transform when any test transitively loads it, and `jest-runtime@30.4.2` has a `clearMocksOnScope` regression that breaks unit tests on some local setups. Came up while opening #234. ## Solution Swap Jest + ts-jest for Vitest + `@vitest/coverage-v8`. The API stays Jest-compatible (`describe`/`it`/`expect`, plus `vi.fn`/`vi.mock` in place of `jest.fn`/`jest.mock`) so the test bodies are mostly unchanged. CI script is identical (`pnpm run test:unit`). Full suite (581 tests) goes from ~5 s under ts-jest to ~1 s with vitest. ## Worth your attention - **`globals: true`** in `vitest.config.ts` keeps `describe`/`it`/`expect` available without import churn; only the four files that used `jest.SpyInstance` / `jest.Mock` types now import `MockInstance` / `Mock` from `'vitest'`. - **Arrow-function `mockImplementation` factories were converted to regular functions** in `factory.test.ts`, `transports.test.ts`, and `keychain.test.ts` — vitest's stricter constructor-call semantics reject arrows as constructors (`new (() => ...)` throws "not a constructor"). Functionally equivalent. - **`vi.hoisted()` in `grep.test.ts`** — vitest hoists `vi.mock` calls above local `const` declarations more aggressively than Jest, so the chalk-identity helpers had to move into a hoisted block. - **`transformIgnorePatterns` for `chalk`/`cli-table3` is gone** — vitest runs ESM natively, so the allow-list is no longer needed. New ESM deps (e.g. `ora`) will Just Work without config tweaks. - Internal-only change — no impact on the published package, no CHANGELOG entry. ## Follow-up - #234 (`feat(cli): add shell tab-completion`) is now stacked on top of this PR. Once this lands, #234 will be ready for review on its own merits.
Member
|
#236 was merged, but pls let's not merge this one quite yet, I also want to have a look |
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
3 participants
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.
Context
Tab-completion was a known gap (
docs/TODOs.md: "Implement typing tab-completions"). Typingmcpc @apify tools-call sea<TAB>did nothing useful and remembering 17 session subcommands is friction. Closes that TODO.Solution
kubectl/gh/cobra-style two-piece pattern:
mcpc completion <bash|zsh|fish>prints a thin shell script (~30 lines), stable across mcpc upgrades — all intelligence lives in TS.mcpc __complete -- <words...> <partial>is the hidden subcommand the script calls on every TAB. Returns candidates on stdout (one per line) plus an optional:<bitflag>directive line.mcpc completion install [shell]auto-detects the user's shell and writes the script to the standard path.Completes top-level commands, session subcommands,
@sessionnames, auth-server hosts, log levels, all known flags, and tool/resource/prompt names for connected sessions.Worth your attention
getCommandFlagMetadata()walkscreateTopLevelProgram()+createSessionProgram()once per process, so adding a new.option(...)inindex.tsshows up in completion automatically. To make this possible those factories are now exported andbin/mcpccallsmod.run()explicitly; a small entry-point guard at the bottom ofindex.tskeepstsx src/cli/index.ts(used by the README-help test) working.cachedToolsmap is unreachable. Tool/resource/prompt names are mirrored to~/.mcpc/completion/<session>.jsonwhenever the user runs the corresponding*-listcommand (awaited so the write lands before exit). TAB never triggers network calls or OAuth flows.-h, --helphardcoded asBUILTIN_HELP_FLAGS— Commander tracks the help option outside its publicoptionsarray and exposes no public getter; rather than reach into_getHelpOption()with a cast, every command in this codebase keeps the default help so a const supplement is reliable.mcpc clean sessionsreaps the cache —deleteCompletionCache(name)runs next todeleteSession(name), so no orphaned cache files build up.createRequire(import.meta.url)('../../package.json')→import pkg from '../../package.json' with { type: 'json' }— minor modernization that happened to fall out of this work.Follow-up
notifications/{tools,resources,prompts}/list_changedso the user doesn't need to runtools-listfirst.