chore: visor-cli updates for logs and init actions

[LKasianAnsys]

Addresses #155

Description

Updates to the 'init' and 'logs' actions in the VISOR CLI tool:

• init
  Add option to start a session in the same command as starting a new VISOR instance. We currently have an action to spin up a new VISOR instance (init) and a separate action to start the visualizer (start). It makes sense to have a dedicated start action for cases where we are loading a dataset and/or metadata into the scene. But for the case where a user wants to spin up a VISOR instance with an empty scene and start it immediately (e.g. where they will be loading a saved state into it), this PR adds a --start flag.
• logs
  Current functionality is maintained but the 'logs' action is given its own parser to take three actions: tail (which is the current default behaviour), list, and a new action, clear (which removes the logs directory). It also selects the visor.log file as a default log to display, which previously a user would need to specify explicitly.

Old behaviour -> New behaviour (defaults to 'visor.log'):
visor-cli logs visor -f -> visor-cli logs tail -f

New functionality (optional -f flag to remove without prompting; default asks for confirmation)
visor-cli logs clear -f

Copilot summary

This pull request introduces several improvements and new features to the Visor CLI, focusing on enhanced log management capabilities and server initialization flexibility. The most significant changes include the addition of a new logs subcommand structure with support for listing, tailing, and clearing log files, as well as an option to start the viewer instance immediately after initialization. Extensive tests have been added and updated to cover these new features and changes.

Log management enhancements:

• Added a new logs subcommand structure with three actions: list (list available logs), tail (tail a log file, with support for -f/--follow and -n/--lines), and clear (delete the log directory with optional confirmation via -f/--force). The corresponding methods (list_logs, tail_log, clear_logs) were implemented in LogsAPI, replacing the previous single log display method. [1] [2] [3] [4]
• Improved log listing output to include the log directory path in messages for better clarity.
• Comprehensive unit tests were added for all new log management features, including confirmation prompts, error handling, and force deletion. [1] [2] [3] [4] [5]

Server initialization improvements:

• Added a --start flag to the server init command, allowing users to optionally start the viewer instance immediately after initialization. This is reflected in argument parsing, the ServerAPI.initialize method signature, and the main CLI logic. [1] [2] [3] [4] [5]
• Updated and added tests to verify that the --start flag is correctly parsed, forwarded, and triggers the expected API calls. [1] [2] [3]

General CLI improvements:

• The CLI now allows log-related commands to run even if the server is not running, improving usability for log inspection and cleanup.

These changes collectively make the CLI more robust, user-friendly, and testable, especially for log management and server initialization workflows.

[Copilot]

Copilot review overview

🟡 Changes recommended

Initialization error handling and log-clearing lifecycle issues can cause incorrect startup or unreliable deletion behavior.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Abort before starting when initialization fails · New
• Preserve start method compatibility with a default argument · New
• Avoid global logging shutdown when clearing active server logs · New

What changed in this PR

Enhances the VISOR CLI with optional immediate startup and expanded log management.

Changes:

• Adds server init --start.
• Adds logs list, tail, and clear actions.
• Expands unit tests and changelog coverage.

File	Description
src/​ansys/​visor/​viewer/​cli/​apis.py	Implements startup and log operations.
src/​ansys/​visor/​viewer/​cli/​parse_args.py	Defines the new CLI options.
src/​ansys/​visor/​viewer/​cli/​visor_cli.py	Dispatches new CLI behavior.
tests/​unit/​cli/​test_apis.py	Tests API implementations.
tests/​unit/​cli/​test_parse_args.py	Tests argument parsing.
tests/​unit/​cli/​test_visor_cli.py	Tests command dispatch.
doc/​changelog.d/​151.maintenance.md	Records the maintenance update.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Server detection and failure propagation can currently permit unsafe deletion or report unsuccessful operations as successful.

Review effort: Balanced
Findings: 1

Open (1)

• Treat any server response as running before deleting logs · New

Resolved since last review (3)

• Abort before starting when initialization fails
• Avoid global logging shutdown when clearing active server logs
• Preserve start method compatibility with a default argument

Previously missed (2)

In code that hasn't changed since last review

Handle /start request failures in initialize

src/​ansys/​visor/​viewer/​cli/​apis.py:91

If the new /start request returns an HTTP error, initialize(..., start=True) still returns normally, so the CLI reports success even though the requested session was not started. Check the second response status just as the initialization response is checked.

Return nonzero when log deletion fails

src/​ansys/​visor/​viewer/​cli/​apis.py:313

A failed rmtree is only printed and then swallowed, causing visor-cli logs clear to exit with status 0 even though nothing was cleared. This makes the force form unsafe for scripts and cleanup automation. Propagate the failure (or return a failure result that main() converts to a nonzero exit code) after reporting it.

[Copilot]

Copilot review overview

🟡 Changes recommended

The clearing guard remains unsafe for unhealthy live servers, and the linked issue does not match the implementation.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Avoid clearing logs when the server is reachable but unhealthy · New
• Incorrectly claims issue #23 is implemented · New

Resolved since last review (1)

• Treat any server response as running before deleting logs

[Copilot]

Copilot review overview

🟡 Changes recommended

Server reachability and failure propagation gaps could permit unsafe clearing or report unsuccessful operations as successful.

Review effort: Balanced
Findings: 1

Open (1)

• Distinguish connection failures from post-connect server errors · New

Resolved since last review (2)

• Avoid clearing logs when the server is reachable but unhealthy
• Incorrectly claims issue #23 is implemented

Previously missed (2)

In code that hasn't changed since last review

Validate HTTP status for the /start response

src/​ansys/​visor/​viewer/​cli/​apis.py:91

The new --start path never calls raise_for_status() on the /start response. If initialization succeeds but starting returns 4xx/5xx, the command exits successfully even though the requested instance was not started. Validate this response just as the initialization response is validated.

Propagate log removal errors to the command exit status

src/​ansys/​visor/​viewer/​cli/​apis.py:313

A removal error is printed and then swallowed, so visor-cli logs clear -f exits with status 0 even when the directory was not cleared. Re-raise the filesystem error (or propagate a failure result to main) so scripts can detect that this destructive operation failed.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Start and log-deletion failures can incorrectly produce successful CLI exits, and the linked issue does not match the changes.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Distinguish connection failures from post-connect server errors

Previously missed (2)

In code that hasn't changed since last review

Check /start response before reporting successful viewer startup

src/​ansys/​visor/​viewer/​cli/​apis.py:91

A 4xx/5xx response from /start is printed but never checked, so visor-cli server init --start can exit successfully even though the viewer failed to start. Check the start response before reporting success.

Propagate log deletion failures and return a nonzero exit status

src/​ansys/​visor/​viewer/​cli/​apis.py:313

Deletion failures are swallowed and the method returns normally, causing visor-cli logs clear to exit with status 0 after permission errors or locked files. Propagate a failure status and have the CLI exit nonzero so scripts can reliably detect that logs were not cleared.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The start request can fail while the CLI exits successfully, and the PR references an unrelated issue.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Check /start HTTP status before reporting successful startup

src/​ansys/​visor/​viewer/​cli/​apis.py:91

The new --start request does not check the HTTP status. If /start returns a 4xx/5xx response, the body is printed and visor-cli server init --start still exits successfully even though the requested instance was not started. Call raise_for_status() for this response as is already done for /initialize.

[Copilot]

Copilot review overview

🟡 Changes recommended

Log clearing can delete active logs if the server starts during the confirmation prompt.

Review effort: Balanced
Findings: 1

Open (1)

• Recheck server reachability before deleting active logs · New

[LKasianAnsys]

@margalva @ansBAkula this copilot comment is a valid point. I think the server check here at the CLI layer is likely sufficient though; the visor-cli logs clear action is for convenience (instead of running rm -r logs manually). Its current server check is not a guarantee it's safe, it's a best-effort guard. But, let me know what you think.

[ansBAkula]

@LKasianAnsys good to have --start flag as part of the server init, but why not make it default True ?

[LKasianAnsys]

@ansBAkula Good question, the main reason I kept the default False is to, by default, keep the Visor class instance instantiation separate from starting the Trame server itself (the original design). The normal workflow is

# Create the new instance on a new port
visor-cli server init --port 8082 
# Start the instance with a dataset 
visor-cli instance start examples/assets/vtk_scene_sphere_l2_b3_r32_v3_c1_z0.vtm --metadata-path examples/assets/vtk_scene_sphere_l2_b3_r32_v3_c1_z0.json

The use case I was picturing for the --start flag was the case where we want to load a state into a blank visualizer. Right now that requires:

# Create the new instance on a new port
visor-cli server init --port 8083
# Start the instance with no dataset
visor-cli instance start
# Load the saved state
visor-cli instance load <saved_state_dir>

So it's more like a convenience helper to optionally start the server in the simple case where no dataset is being loaded into the scene on the 'start' API call, but the default usage would keep init separate from start.