Shell completions for Bash, Zsh # 128#192
Conversation
| ] | ||
| dependencies = [ | ||
| "moss>=1.0.0", | ||
| "moss==1.0.0", |
| except RuntimeError as e: | ||
| if "moss_core" in str(e) and "not available" in str(e): | ||
| pass # management API unavailable; save without validation | ||
| else: | ||
| console.print(f"[red]Authentication failed: {e}[/red]") | ||
| save_anyway = Prompt.ask("Save anyway?", choices=["y", "n"], default="n") | ||
| if save_anyway != "y": | ||
| raise typer.Exit(1) |
There was a problem hiding this comment.
Why is this exception necessary ?
| def version_command(ctx: typer.Context) -> None: | ||
| """Print CLI and SDK version information.""" | ||
| import moss_cli | ||
| import moss | ||
|
|
||
| try: | ||
| import moss | ||
| sdk_version = moss.__version__ | ||
| except Exception: | ||
| sdk_version = "unavailable" | ||
|
|
||
| json_mode = ctx.obj.get("json_output", False) | ||
|
|
||
| if json_mode: | ||
| import json | ||
|
|
||
| print(json.dumps({"cli": moss_cli.__version__, "sdk": moss.__version__})) | ||
| print(json.dumps({"cli": moss_cli.__version__, "sdk": sdk_version})) | ||
| else: | ||
| console.print(f"moss-cli {moss_cli.__version__}") | ||
| console.print(f"moss SDK {moss.__version__}") | ||
| console.print(f"moss SDK {sdk_version}") |
There was a problem hiding this comment.
Please try to keep the change as relevant and minimal as possible.
| except Exception: | ||
| pass # moss SDK unavailable; SDK-dependent commands will be absent |
There was a problem hiding this comment.
🔴 Overly broad except Exception silently swallows non-import errors during command registration
The except Exception: pass block at main.py:44-45 is intended to catch ImportError when the moss SDK is not installed, but it catches all exceptions. If any of the SDK-dependent command modules (doc.py, index.py, init_cmd.py, job.py, search.py) have a runtime error at import time or during registration (e.g., AttributeError, TypeError, NameError, SyntaxError), those errors are silently swallowed. The affected commands simply disappear from the CLI with no warning or error message, making such failures extremely hard to diagnose. Since all five modules only depend on from moss import ... at the top level, catching ImportError (or ModuleNotFoundError) would be sufficient and safe.
Was this helpful? React with 👍 or 👎 to provide feedback.
…and init handler - Revert moss==1.0.0 back to moss>=1.0.0 to allow patch updates - Remove unnecessary RuntimeError handling for moss_core in init command - Remove unnecessary try/except around moss import in version command - Move _register_moss_commands to module level so app is fully populated for tests
| no_args_is_help=True, | ||
| ) |
There was a problem hiding this comment.
🚩 Typer's built-in completion (add_completion=True) coexists with custom completions command
The app = typer.Typer(add_completion=True) at main.py:17 adds Typer's built-in --install-completion and --show-completion global options. The new completions subcommand provides richer, hand-crafted shell scripts. These don't technically conflict (one is global options, the other a named command), but having two completion mechanisms could confuse users. Consider setting add_completion=False to avoid ambiguity, or document the distinction.
(Refers to lines 17-20)
Was this helpful? React with 👍 or 👎 to provide feedback.
| def _register_moss_commands() -> None: | ||
| """Import and register commands that require the moss SDK.""" | ||
| from .commands.doc import doc_app | ||
| from .commands.index import index_app | ||
| from .commands.init_cmd import init_command | ||
| from .commands.job import job_app | ||
| from .commands.search import query_command | ||
|
|
||
| app.add_typer(index_app, name="index", help="Manage indexes") | ||
| app.add_typer(doc_app, name="doc", help="Manage documents") | ||
| app.add_typer(job_app, name="job", help="Track background jobs") | ||
| app.command(name="query")(query_command) | ||
| app.command(name="init")(init_command) | ||
|
|
||
| try: | ||
| _register_moss_commands() | ||
| except Exception: | ||
| pass # moss SDK unavailable; SDK-dependent commands will be absent |
There was a problem hiding this comment.
🚩 No tests added for new completions command or lazy-loading pattern
The PR adds a new completions_command function and a significant behavioral change (lazy registration of SDK-dependent commands). Neither has accompanying tests. The CONTRIBUTING.md guideline states "If you've added code that should be tested, add tests." While the completions function is simple (echoing static strings), the lazy-loading pattern in main.py is a meaningful behavioral change that would benefit from at least a smoke test verifying commands register correctly and degrade gracefully.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@yatharthk2 Sir I have made changes according to the comments I got |
…d lazy-loading - Set add_completion=False to avoid duplicate completion mechanisms (Typer built-in vs custom 'moss completions' command) - Add test_completions.py with 7 tests covering: - Bash/Zsh script output - Missing/invalid shell argument errors - SDK-dependent command registration (lazy-loading) - Help text display - Verification that --install-completion/--show-completion are removed Resolves PR usemoss#192 review comments.
|
@yatharthk2 updated pr |
Screencast.from.2026-04-30.18-22-22.webm
Implementation (completions.py): Added a new top-level moss completions [bash|zsh] command.
Full Command Hierarchy Coverage: The generated shell scripts explicitly define auto-completion logic for all CLI commands (index, doc, job, profile, query, init, version) and their respective subcommands (create, list, get, delete, etc.).
Flag Auto-Completion: Automatically completes contextual flags (e.g., --file, --model, --wait, --poll-interval, --profile) depending on the current subcommand you are using.
Dynamic Index Name Resolution: Embedded a helper function in both the Bash and Zsh scripts that dynamically fetches your available indexes in the background using moss index list --json. This means commands like moss query or moss doc add will automatically populate with your actual remote index names.
2. CLI Registration (main.py)
Hooked the completions command into the main Typer application.
Registered it alongside commands that do not depend on the Moss SDK (like version), ensuring that users can always generate completion scripts regardless of their local environment setup.
3. Graceful SDK Fallbacks (init_cmd.py & version.py)
Init Command Fallback: Updated the init command so that if the moss_core native binary is missing (which makes the Management API unavailable), it handles the RuntimeError gracefully and saves your credentials without forcing validation, preventing the initialization process from hard crashing.
Version Command Resiliency: Updated version.py so that if the moss package fails to import, it catches the exception and simply reports the SDK version as "unavailable" rather than breaking the entire moss version output.