feat(ows-cli): add 'ows doctor' diagnostic command for vault health checks#164
Open
okwn wants to merge 9 commits intoopen-wallet-standard:mainfrom
Open
feat(ows-cli): add 'ows doctor' diagnostic command for vault health checks#164okwn wants to merge 9 commits intoopen-wallet-standard:mainfrom
okwn wants to merge 9 commits intoopen-wallet-standard:mainfrom
Conversation
Design document for the `ows doctor` diagnostic command covering: - CLI position (top-level command alongside Update/Uninstall) - V1 check list (vault path, existence, permissions, config/wallet/key/policy parsing) - Internal model (DoctorStatus, DoctorFinding, DoctorReport) - 4-state taxonomy (Pass/Warn/Fail/Skip) with exit code rules - Human-readable output shape - Testing plan and V1 out-of-scope items - File change plan for implementation phases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 2: Internal diagnostic domain model and execution framework (CLI wiring comes in Phase 3) - `report.rs`: DoctorStatus (Ok/Warning/Error/Skipped), DoctorFinding (id, status, title, detail, suggestion, path, code), DoctorReport (findings, summary counts, overall_status, exit_code), DoctorCheckId - `checks.rs`: Individual check implementations for vault path, vault existence, wallets/keys/policies/logs subdirectories, config parse, Unix permissions. Each check is a pure function returning Vec<DoctorFinding>. resolve_vault_path() helper avoids env var dependence in tests. - `mod.rs`: Module docs, re-exports, DoctorResult alias - `commands/mod.rs`: Added `pub mod doctor;` - 16 passing unit tests covering: empty vault, missing subdirs, malformed config, permission warnings, healthy state, report aggregation and exit code logic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 3: Read-only vault inspection for wallet, key, and policy artifacts
- vault_inspector.rs: New module with check_wallet_files(),
check_key_files(), check_policy_files(). Each function:
- Enumerates .json files in the vault subdirectory
- Reads and parses each file as the appropriate type
- Validates metadata where safe (empty ID, invalid created_at)
- Returns granular findings per file plus summary finding
- Error codes: ERR_FILE_UNREADABLE, ERR_FILE_MALFORMED,
ERR_METADATA_INVALID, WARN_NO_WALLETS, WARN_ARTIFACTS_CORRUPTED
- checks.rs: Removed redundant dir-check functions; delegates to
vault_inspector from run_all_checks(). Resolves vault path once
and passes it to all artifact checks.
- 13 new unit tests covering: valid fixtures, malformed JSON,
empty directories, empty ID, invalid created_at, mixed
valid/corrupt artifacts
- Separation maintained: checks.rs handles structural/env checks,
vault_inspector handles artifact content validation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 4 completes the `ows doctor` implementation by: - Adding `Doctor` command to the CLI parser in main.rs - Wiring `doctor::run()` to execute all checks and print the report - Adding human-readable output formatter with status grouping - Fixing test_wallet_files_empty_dir_is_warning (create_dir_all fix) - Adding doctor section to sdk-cli.md documentation Exit code 0: all checks pass or only warnings found Exit code 1: errors detected (diagnostic checks failed) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phase 5 makes diagnostics consistent, actionable, and maintainable: - Add 23 stable `OWS_DOCTOR_*` constants as the canonical finding codes: OWS_DOCTOR_ENV_HOME_NOT_SET, OWS_DOCTOR_VAULT_MISSING, OWS_DOCTOR_LOGS_DIR_MISSING, OWS_DOCTOR_CONFIG_MISSING, OWS_DOCTOR_CONFIG_INVALID, OWS_DOCTOR_DIR_UNREADABLE, OWS_DOCTOR_PERM_VAULT_INSECURE, OWS_DOCTOR_PERM_WALLETS_INSECURE, OWS_DOCTOR_PERM_WALLET_FILE_INSECURE, OWS_DOCTOR_WALLET_NONE, OWS_DOCTOR_WALLET_FILE_UNREADABLE, OWS_DOCTOR_WALLET_FILE_INVALID, OWS_DOCTOR_WALLET_METADATA_CORRUPT, OWS_DOCTOR_WALLET_SOME_CORRUPT, OWS_DOCTOR_POLICY_NONE, OWS_DOCTOR_POLICY_FILE_UNREADABLE, OWS_DOCTOR_POLICY_FILE_INVALID, OWS_DOCTOR_POLICY_SOME_CORRUPT, OWS_DOCTOR_KEY_NONE, OWS_DOCTOR_KEY_FILE_UNREADABLE, OWS_DOCTOR_KEY_FILE_INVALID, OWS_DOCTOR_KEY_SOME_CORRUPT - Change DoctorFinding builders: skipped/warning/error now require a stable code as the second argument. ok() remains code-free. - Every Error, Warning, and Skipped finding now carries a machine-stable code suitable for future JSON output and programmatic handling. - Wording polished: operator-facing titles and detail text revised for clarity, precision, and actionability. Suggestions updated to be concrete and CLI-ready. - Tests updated: all 31 doctor tests assert stable codes where appropriate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add `ows doctor` to cli-reference.md partial (source-of-truth) - Run readme/generate.sh to propagate to all generated READMEs - Update docs/sdk-cli.md with Phase 5 examples: - Add healthy vault sample output - Add missing vault sample output - Add corrupted wallet file example - Add platform caveats for permission checks (Unix-only) - Document exit code semantics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Apply `cargo fmt` to the doctor module. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@okwn is attempting to deploy a commit to the MoonPay Team on Vercel. A member of the Team first needs to authorize it. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| CHECK_VAULT_PERMS, | ||
| "Permissions check passed", | ||
| "All vault and wallet permissions are correct.", | ||
| )] |
There was a problem hiding this comment.
Permissions check reports false OK on metadata failure
Low Severity
The check_vault_permissions function falls back to reporting "All vault and wallet permissions are correct" when findings is empty. This can happen if std::fs::metadata(&vault_path) fails (line 190 if let Ok silently skips) even though vault_path.exists() returned true. In that scenario, no permissions were actually verified but the function reports a passing result.
njdawn
requested changes
Apr 3, 2026
Contributor
njdawn
left a comment
njdawn
left a comment
There was a problem hiding this comment.
Status: merge blocker
Findings:
Blocker — ows/crates/ows-cli/src/commands/doctor/checks.rs: Unix build fails with E0716 because a borrowed file_name outlives the temporary created by entry.file_name().to_string_lossy(). This needs an owned string before formatting/output.
Testing:
cargo check -p ows-cli failed with the borrow checker error above.
Broader doctor integration tests were not run.
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
2 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.
What was added
This PR adds a new top-level
ows doctorcommand that performs read-only diagnostic checks on the local OWS installation and vault health.The command does not mutate, repair, or modify any files.
Checks performed
config.jsonvalidity via existing config loading/parsingExit code semantics
0— all checks passed, or only warnings were foundImplementation summary
ows doctorCLI commandOWS_DOCTOR_*finding codesImportant constraints
ows doctoris strictly read-onlyTests run
cargo fmt --all -- --checkcargo check -p ows-clicargo test -p ows-clicargo test -p ows-clipasses with all doctor-related tests included.Notes for reviewers
cargo test --workspacestill shows 2 pre-existing Windows-only failures inows-libpolicy engine tests; these are unrelated to this PR and were not introduced by this branch.runtime_depscheck mentioned in the design note was intentionally not implemented in this PR because it does not currently provide a clear actionable remediation path for users.Why this is useful
This improves DX by giving users and automation a first-party way to diagnose:
without changing any signing or wallet behavior.
Note
Low Risk
Low risk: adds a new read-only CLI command and documentation; main behavioral change is additional command routing and local file/permission inspection with no mutation of vault data.
Overview
Adds a new top-level
ows doctorcommand to the Rust CLI that runs read-only diagnostics against the local vault and prints a grouped human-readable report, returning a non-zero exit code when any Error finding is present.Introduces a small diagnostic framework (
DoctorReport/DoctorFinding/DoctorStatusplus stableOWS_DOCTOR_*codes) and implements checks for vault path/HOME, vault +logs/+config.jsonpresence/parseability, wallet/key/policy JSON integrity (including wallet metadata validation), and Unix-only permission checks (skipped on non-Unix).Updates CLI entrypoints (
commands/mod.rs,main.rs) and public docs/READMEs to include the new command and usage examples.Written by Cursor Bugbot for commit bf8ef83. This will update automatically on new commits. Configure here.