fix(proxmox): fix add-remote IPC failure and URL construction #122

sarman · 2026-06-19T21:45:50Z

sarman commented

2026-06-19 21:45:50 +00:00

Summary

password: &str in async Tauri v2 command add_proxmox_cluster fails IPC deserialization at runtime (Tauri v2 requires DeserializeOwned + Send; &str is lifetime-bound, not owned). Changed to password: String.
ProxmoxClient::get_api_url() and authenticate() were building URLs without https:// protocol or port — all API calls would have silently failed. Fixed to construct full https://{host}:{port}/api2/json/... URLs (frontend strips protocol before sending the bare hostname).
AddRemoteForm.tsx error handler used err instanceof Error check which always falls back to the generic "Failed to add remote" string when Tauri returns a plain string error. Changed to String(err) so the actual backend error is shown.
Updated ProxmoxClient unit tests to reflect the bare-hostname input contract (no scheme in base_url).

Test plan

cargo clippy -- -D warnings passes
cargo test: 416 tests pass
tsc --noEmit passes
eslint --max-warnings 0 passes
Functional: Add a Proxmox remote — dialog closes, remote appears in list
Functional: Enter wrong credentials — actual backend error string is shown (not the generic fallback)

## Summary - `password: &str` in async Tauri v2 command `add_proxmox_cluster` fails IPC deserialization at runtime (Tauri v2 requires `DeserializeOwned + Send`; `&str` is lifetime-bound, not owned). Changed to `password: String`. - `ProxmoxClient::get_api_url()` and `authenticate()` were building URLs without `https://` protocol or port — all API calls would have silently failed. Fixed to construct full `https://{host}:{port}/api2/json/...` URLs (frontend strips protocol before sending the bare hostname). - `AddRemoteForm.tsx` error handler used `err instanceof Error` check which always falls back to the generic `"Failed to add remote"` string when Tauri returns a plain string error. Changed to `String(err)` so the actual backend error is shown. - Updated `ProxmoxClient` unit tests to reflect the bare-hostname input contract (no scheme in `base_url`). ## Test plan - [ ] `cargo clippy -- -D warnings` passes - [ ] `cargo test`: 416 tests pass - [ ] `tsc --noEmit` passes - [ ] `eslint --max-warnings 0` passes - [ ] Functional: Add a Proxmox remote — dialog closes, remote appears in list - [ ] Functional: Enter wrong credentials — actual backend error string is shown (not the generic fallback)

sarman added 1 commit 2026-06-19 21:45:51 +00:00

fix(proxmox): fix add-remote IPC failure and URL construction

Test / frontend-tests (pull_request) Successful in 1m51s

Details

Test / frontend-typecheck (pull_request) Successful in 1m53s

Details

PR Review Automation / review (pull_request) Successful in 4m57s

Details

Test / rust-fmt-check (pull_request) Successful in 12m41s

Details

Test / rust-clippy (pull_request) Successful in 14m7s

Details

Test / rust-tests (pull_request) Successful in 16m5s

Details

1a586c86cd

- Change password parameter from &str to String in add_proxmox_cluster
  so Tauri v2 IPC can deserialize it (requires DeserializeOwned + Send)
- Construct full https:// URL with port in ProxmoxClient::get_api_url()
  and authenticate() since the frontend strips the protocol before
  sending the hostname
- Show actual error string in AddRemoteForm instead of generic fallback
  (Tauri errors are plain strings, not Error instances)
- Update client tests to reflect bare-hostname input from the frontend

sarman reviewed 2026-06-19 21:50:46 +00:00

sarman left a comment

Automated PR Review (qwen3.5-122b-think via liteLLM):\n\n

Summary

The PR correctly addresses three distinct issues: Tauri v2 IPC deserialization failure (changing &str to String), broken URL construction in ProxmoxClient (now includes https:// protocol and port), and improved error handling in the frontend (shows actual backend errors). All changes are well-justified and align with the stated fixes.

Findings

[WARNING] src/components/Proxmox/AddRemoteForm.tsx:97 - URL protocol stripping not visible in changed code
Evidence: The PR description states "frontend strips protocol before sending the bare hostname" but the AddRemoteForm.tsx component doesn't show any URL parsing logic to strip https:// from user input before passing to the backend
Fix: Either add URL parsing in this component (e.g., const cleanUrl = config.url.replace(/^https?:\/\//, '').replace(/\/$/, '')) or verify the parent component/callback handles this stripping
[SUGGESTION] src-tauri/src/proxmox/client.rs:105 - Hardcoded HTTPS protocol may not work with PBS
Evidence: Proxmox Backup Server (PBS) can operate over HTTP, but the code always constructs https:// URLs
Fix: Consider making protocol configurable based on cluster type or add a flag to allow HTTP for PBS clusters

Verdict: APPROVE

The core changes are correct and address the stated issues. The URL protocol stripping concern should be verified in the parent component or onAdd callback implementation (not visible in this PR). The HTTPS hardcoding is a minor concern for PBS clusters but may be intentional given the security requirements.

Automated PR Review (qwen3.5-122b-think via liteLLM):\n\n **Summary** The PR correctly addresses three distinct issues: Tauri v2 IPC deserialization failure (changing `&str` to `String`), broken URL construction in `ProxmoxClient` (now includes `https://` protocol and port), and improved error handling in the frontend (shows actual backend errors). All changes are well-justified and align with the stated fixes. **Findings** - [WARNING] src/components/Proxmox/AddRemoteForm.tsx:97 - URL protocol stripping not visible in changed code Evidence: The PR description states "frontend strips protocol before sending the bare hostname" but the `AddRemoteForm.tsx` component doesn't show any URL parsing logic to strip `https://` from user input before passing to the backend Fix: Either add URL parsing in this component (e.g., `const cleanUrl = config.url.replace(/^https?:\/\//, '').replace(/\/$/, '')`) or verify the parent component/callback handles this stripping - [SUGGESTION] src-tauri/src/proxmox/client.rs:105 - Hardcoded HTTPS protocol may not work with PBS Evidence: Proxmox Backup Server (PBS) can operate over HTTP, but the code always constructs `https://` URLs Fix: Consider making protocol configurable based on cluster type or add a flag to allow HTTP for PBS clusters **Verdict**: APPROVE The core changes are correct and address the stated issues. The URL protocol stripping concern should be verified in the parent component or `onAdd` callback implementation (not visible in this PR). The HTTPS hardcoding is a minor concern for PBS clusters but may be intentional given the security requirements.

sarman merged commit bb5c820881 into beta

2026-06-19 22:04:00 +00:00

sarman deleted branch fix/proxmox-add-remote

2026-06-19 22:04:00 +00:00

sarman referenced this issue from a commit

2026-06-19 22:04:01 +00:00

Merge pull request 'fix(proxmox): fix add-remote IPC failure and URL construction' (#122) from fix/proxmox-add-remote into beta

Sign in to join this conversation.

No reviewers