Skip to content

fix(cli): return actionable resource errors for fs commands#1458

Merged
MaojiaSheng merged 5 commits intovolcengine:mainfrom
ehz0ah:fix/cli-resource-error
Apr 16, 2026
Merged

fix(cli): return actionable resource errors for fs commands#1458
MaojiaSheng merged 5 commits intovolcengine:mainfrom
ehz0ah:fix/cli-resource-error

Conversation

@ehz0ah
Copy link
Copy Markdown
Contributor

@ehz0ah ehz0ah commented Apr 14, 2026

Description

Fix the CLI/resource error UX for common filesystem states so ov rm, ov abstract, and ov overview return actionable messages instead of generic internal server errors.

This change keeps the existing command logic but improves how backend and router exceptions are translated and surfaced to users. It also preserves structured error codes in the Rust CLI output.

A follow-up test-only commit broadens the server-side regression suite across related filesystem/content endpoints.

Related Issue

Fixes #1457

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • openviking/server/error_mapping.py: add shared exception translation for AGFS/storage/runtime exceptions into structured OpenViking API errors.
  • openviking/server/app.py and openviking/server/routers/filesystem.py: map unhandled filesystem errors to user-facing NOT_FOUND, FAILED_PRECONDITION, CONFLICT, and UNAVAILABLE responses instead of generic 500s.
  • openviking/storage/viking_fs.py: tighten rm, abstract, and overview behavior so not-found, file-vs-directory misuse, backend failures, and non-recursive directory deletes are reported clearly and do not look like internal server bugs.
  • crates/ov_cli/src/client.rs and crates/ov_cli/src/error.rs: preserve structured server error codes in CLI output and remove the redundant generic API error: wrapping in the converted CLI message.
  • crates/ov_cli/src/main.rs: clarify that abstract and overview expect a directory URI.
  • tests/server/test_api_content.py and tests/server/test_api_filesystem.py: add targeted regression coverage for missing resources, file-vs-directory misuse, resource-busy conflicts, backend unavailability, and directory deletion without --recursive.
  • tests/server/test_api_fs_content_endpoint_suite.py: add broader endpoint coverage for fs/ls, fs/tree, fs/stat, fs/mkdir, fs/mv, content/read, content/download, content/write, and content/reindex.
  • tests/server/test_server_health.py and tests/server/test_error_scenarios.py: tighten existing not-found assertions from permissive 404 or 500 checks to strict structured 404 expectations.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Validation run locally:

uv run pytest tests/server/test_api_content.py tests/server/test_api_content_write.py tests/server/test_api_filesystem.py tests/server/test_api_fs_content_endpoint_suite.py tests/server/test_server_health.py tests/server/test_error_scenarios.py
cargo check -p ov_cli
cargo fmt --check -p ov_cli

Result:

  • 55 passed for the expanded server test suite
  • cargo check -p ov_cli passes
  • cargo fmt --check -p ov_cli passes

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Not applicable.

Representative CLI behavior after the fix:

$ ov rm viking://resources/codex-e2e-sample
Error: [FAILED_PRECONDITION] Cannot remove directory without --recursive: viking://resources/codex-e2e-sample

$ ov abstract viking://resources/codex-e2e-sample
Error: [NOT_FOUND] Resource not found: viking://resources/codex-e2e-sample

$ ov overview viking://resources/codex-e2e-sample
Error: [NOT_FOUND] Resource not found: viking://resources/codex-e2e-sample

Additional Notes

  • The original report came from user feedback about confusing CLI resource errors.
  • This PR now includes a broader endpoint regression suite for related filesystem/content behavior, while keeping the runtime fix itself focused on the original CLI resource-error path.
  • The broader test suite is scoped to the filesystem/content endpoints most related to this bug class.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1457 - Partially compliant

Compliant requirements:

  • All reported issues are addressed with proper error mapping and user-facing messages
  • Added regression tests for the fixed scenarios
  • Preserved structured error codes in CLI output

Non-compliant requirements:

  • (No unfulfilled requirements identified)
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 92
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Missing License Header

New Python file missing required AGPL-3.0 license header. All new files in openviking/ must include the standard copyright and license header for compliance.

from openviking.pyagfs.exceptions import (

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@MaojiaSheng
Copy link
Copy Markdown
Collaborator

#1428 fixed this

@ehz0ah
Copy link
Copy Markdown
Contributor Author

ehz0ah commented Apr 15, 2026

#1428 fixed this

#1428 only seems to improves specifically not-found error handling, the underlying problem is that the server
returns generic INTERNAL errors for normal resource states, which affects any API client. The CLI output is just one symptom.

@MaojiaSheng
Copy link
Copy Markdown
Collaborator

#1428 fixed this

#1428 only seems to improves specifically not-found error handling, the underlying problem is that the server returns generic INTERNAL errors for normal resource states, which affects any API client. The CLI output is just one symptom.

OK, thanks, and could you change the code after #1428, and fixes the conflicts

…urce-error

# Conflicts:
#	openviking/server/routers/filesystem.py
@MaojiaSheng MaojiaSheng merged commit d13bf58 into volcengine:main Apr 16, 2026
10 of 11 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: CLI resource commands return Internal server error for normal resource states

2 participants