Skip to content

feat(cli): enhance ov --version to show both CLI and server versions#835

Open
ZaynJarvis wants to merge 2 commits intomainfrom
feat/enhance-ov-version
Open

feat(cli): enhance ov --version to show both CLI and server versions#835
ZaynJarvis wants to merge 2 commits intomainfrom
feat/enhance-ov-version

Conversation

@ZaynJarvis
Copy link
Collaborator

@ZaynJarvis ZaynJarvis commented Mar 21, 2026

Summary

  • Display CLI version with a clear CLI version: label
  • Fetch and display server version from the /health endpoint
  • Handle server-unavailable case gracefully with a config path hint for troubleshooting
  • Prevents user confusion when CLI and server versions differ

Changes

> ov --version
CLI version: 0.2.6
Server version: 0.2.9

Test plan

  • Run ov --version with server running — should show both CLI version: and Server version:
  • Run ov --version with server stopped — should show Server: not found and Check ovcli.conf: <path>
  • Verify OPENVIKING_CLI_CONFIG_FILE env var overrides the config path hint

- Display CLI version with "CLI version:" label for clarity
- Fetch server version from /health endpoint and display it
- Handle server unavailable case with helpful config path hint
- Show environment-aware config file path for troubleshooting
- Prevents user confusion about CLI vs server version mismatch
@github-actions
Copy link

Failed to generate code suggestions for PR

Override clap's built-in --version/-V to also fetch and display the
server version from /health endpoint. Shows a config path hint when
the server is unreachable, preventing CLI vs server version confusion.
Copy link
Collaborator

@qin-ctx qin-ctx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One blocking issue found — misleading error message when config loading fails in the --version handler.

}
}
} else {
println!("Server: not found");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Bug] (blocking) When CliContext::new() fails (most commonly because the config file is missing or malformed), this branch prints "Server: not found", which is misleading — the actual problem is that the config couldn't be loaded, so the CLI doesn't even know the server's address.

Additionally, unlike the Err(_) branch on line 538 which shows the config path hint, this branch omits it — precisely in the scenario (missing config) where the hint is most needed.

Suggested fix:

} else {
    println!("Server: config not loaded");
    let config_path = if let Ok(env_path) = std::env::var("OPENVIKING_CLI_CONFIG_FILE") {
        env_path
    } else {
        match config::default_config_path() {
            Ok(path) => path.to_string_lossy().to_string(),
            Err(_) => "~/.openviking/ovcli.conf".to_string(),
        }
    };
    println!("Check ovcli.conf: {}", config_path);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants