Merge pull request 'fix: resolve Proxmox authentication response parsing error' (#126) from fix/proxmox-auth-decoding into beta

Reviewed-on: #126

.opencode/openwork.json

@@ -0,0 +1,12 @@
+{
+  "version": 1,
+  "workspace": {
+    "name": "tftsr-devops_investigation",
+    "createdAt": 1781879156243,
+    "preset": "starter"
+  },
+  "authorizedRoots": [
+    "/home/sarman/Documents/tftsr-devops_investigation"
+  ],
+  "reload": null
+}

INVESTIGATION_FINDINGS.md

@@ -0,0 +1,231 @@
+# Investigation Findings: Remote Add Failure and Updater Errors
+
+## Executive Summary
+
+This investigation identified **two critical issues** in the Tauri application:
+
+1. **Remote add failure**: The `add_proxmox_cluster` command is defined and registered, but the error "Failed to add remote" occurs due to missing error handling in the frontend.
+2. **Updater errors**: The updater commands (`check_app_updates`, `get_update_channel`, `set_update_channel`) are **NOT registered** in the Tauri command handler, causing "Command not found" errors.
+
+---
+
+## Issue 1: Remote Add Failure
+
+### Root Cause Analysis
+
+The error "Failed to add remote" occurs in `/src/pages/Proxmox/RemotesPage.tsx` at line 64 and is propagated from the backend `add_proxmox_cluster` command.
+
+#### Code Flow:
+
+1. **Frontend**: `RemotesPage.tsx` → `handleAddRemote()` (line 55-76)
+   - Calls `addProxmoxCluster()` from `proxmoxClient.ts`
+   - Catches error and displays toast notification
+
+2. **Frontend Wrapper**: `proxmoxClient.ts` (line 17-33)
+   ```typescript
+   export async function addProxmoxCluster(
+     id: string,
+     name: string,
+     clusterType: ClusterType,
+     connection: { url: string; port: number },
+     username: string,
+     password: string
+   ): Promise<ClusterInfo> {
+     return await invoke<ClusterInfo>("add_proxmox_cluster", {
+       id, name, cluster_type: clusterType, connection, username, password
+     });
+   }
+   ```
+
+3. **Backend Command**: `src-tauri/src/commands/proxmox.rs` (line 35-105)
+   - **IS registered** in `lib.rs` at line 226
+   - Stores credentials encrypted in database
+   - Creates in-memory client pool (unauthenticated)
+
+#### Current Implementation Status:
+
+- ✅ Command is registered: `commands::proxmox::add_proxmox_cluster` (line 226)
+- ✅ Command implementation exists in `proxmox.rs`
+- ✅ Database migration 034 added `username` column
+- ⚠️ Error handling: Frontend catches and displays error, but backend error details may not be descriptive
+
+#### Recent Changes (Commit 87ccbb64):
+- Removed live authentication requirement during cluster add
+- Credentials are now stored encrypted and used on first API call
+- Added `username` column to database schema
+
+---
+
+## Issue 2: Updater Errors
+
+### Root Cause Analysis
+
+The updater commands are **DEFINED** but **NOT REGISTERED** in the Tauri command handler.
+
+#### Missing Commands:
+
+| Command | Location | Status |
+|---------|----------|--------|
+| `check_app_updates` | `system.rs:498` | ✅ Defined ❌ NOT REGISTERED |
+| `get_update_channel` | `system.rs:589` | ✅ Defined ❌ NOT REGISTERED |
+| `set_update_channel` | `system.rs:598` | ✅ Defined ❌ NOT REGISTERED |
+| `install_app_updates` | `system.rs:579` | ✅ Defined ❌ NOT REGISTERED |
+
+#### Current Command Registration:
+
+**File**: `src-tauri/src/lib.rs` (lines 243-258)
+
+```rust
+// System / Settings
+commands::system::check_ollama_installed,
+commands::system::get_ollama_install_guide,
+commands::system::list_ollama_models,
+commands::system::pull_ollama_model,
+commands::system::delete_ollama_model,
+commands::system::detect_hardware,
+commands::system::recommend_models,
+commands::system::get_settings,
+commands::system::update_settings,
+commands::system::get_audit_log,
+commands::system::get_app_version,
+commands::system::set_sudo_password,
+commands::system::get_sudo_config_status,
+commands::system::test_sudo_password,
+commands::system::clear_sudo_password,
+```
+
+**MISSING**:
+- `commands::system::check_app_updates`
+- `commands::system::get_update_channel`
+- `commands::system::set_update_channel`
+- `commands::system::install_app_updates`
+
+#### Frontend Usage:
+
+**File**: `src/lib/tauriCommands.ts` (lines 652-662)
+
+```typescript
+export const checkAppUpdatesCmd = async (): Promise<UpdateCheckResult> =>
+  invoke<UpdateCheckResult>("check_app_updates");
+
+export const installAppUpdatesCmd = async (): Promise<void> =>
+  invoke<void>("install_app_updates");
+
+export const getUpdateChannelCmd = async (): Promise<string> =>
+  invoke<string>("get_update_channel");
+
+export const setUpdateChannelCmd = async (channel: string): Promise<void> =>
+  invoke<void>("set_update_channel", { channel });
+```
+
+**File**: `src/pages/Settings/Updater.tsx` (lines 32, 52)
+- Calls `checkAppUpdatesCmd()` and `setUpdateChannelCmd()`
+- Both will fail with "Command not found" error
+
+#### Recent Changes (Commit 87ccbb64):
+
+**Before**: Used `tauri-plugin-updater` with `app.updater().check()`
+
+**After**: Direct Gitea HTTP API call (lines 498-576 in `system.rs`)
+
+```rust
+let response = client
+    .get(
+        "https://gogs.tftsr.com/api/v1/repos/sarman/tftsr-devops_investigation/releases?limit=20",
+    )
+    .header("Accept", "application/json")
+    .send()
+    .await
+```
+
+Key improvements:
+- Added update channel filtering (stable vs beta)
+- Returns full release info (version, URL, notes)
+- Uses `tauri-plugin-opener` for browser launch
+
+---
+
+## Files to Modify
+
+### Fix 1: Updater Commands Registration
+
+**File**: `src-tauri/src/lib.rs`
+
+**Location**: Lines 253-258 (after `get_app_version`)
+
+**Required Addition**:
+```rust
+commands::system::check_app_updates,
+commands::system::install_app_updates,
+commands::system::get_update_channel,
+commands::system::set_update_channel,
+```
+
+### Fix 2: Remote Add Error Handling (Optional Enhancement)
+
+**File**: `src-tauri/src/commands/proxmox.rs`
+
+**Location**: Lines 35-105 (`add_proxmox_cluster`)
+
+**Current Behavior**: Returns generic error from database or encryption failures
+
+**Recommended Enhancement**: Add more descriptive error messages for:
+- Invalid URL format
+- Duplicate remote name
+- Database connection issues
+- Encryption failures
+
+---
+
+## Verification Steps
+
+After applying fixes:
+
+1. **Build the application**:
+   ```bash
+   cd src-tauri
+   cargo build --release
+   ```
+
+2. **Test updater commands**:
+   - Navigate to Settings > Updater
+   - Verify channel selection works
+   - Verify "Check Now" button works
+   - Check browser opens on "Download Update"
+
+3. **Test remote add**:
+   - Navigate to Remotes page
+   - Click "Add Remote"
+   - Fill in configuration
+   - Verify remote is added successfully
+   - Check database entry in `proxmox_clusters` table
+
+---
+
+## Related Files
+
+| File | Purpose |
+|------|---------|
+| `src-tauri/src/lib.rs` | Command registration (MISSING updater commands) |
+| `src-tauri/src/commands/system.rs` | Updater command implementations |
+| `src-tauri/src/commands/proxmox.rs` | Remote/cluster command implementation |
+| `src/lib/tauriCommands.ts` | Frontend command wrappers |
+| `src/lib/proxmoxClient.ts` | Proxmox API client |
+| `src/pages/Settings/Updater.tsx` | Updater UI |
+| `src/pages/Proxmox/RemotesPage.tsx` | Remote management UI |
+| `src/components/Proxmox/AddRemoteForm.tsx` | Remote add form |
+
+---
+
+## Git History Context
+
+**Commit 87ccbb64** ("fix(proxmox): remove dummy data, fix add-remote, fix updater"):
+- Added `list_proxmox_containers` command
+- Fixed `add_proxmox_cluster` to store credentials without live auth
+- Replaced `tauri-plugin-updater` with direct Gitea API
+- Added update channel filtering
+- **Issue**: Forgot to register updater commands in `lib.rs`
+
+**Current Branch**: `fix/proxmox-v1.2.1` (commit 758d783e)
+
+**Latest Release**: v1.2.3 (commit 8befa472)

PR_REVIEW_RESPONSE.md

@@ -18,7 +18,7 @@ Created `parseRemoteUrl()` helper function to eliminate code duplication:
  * Helper function to parse a Proxmox URL and extract hostname and port.
  * Handles URLs with or without explicit port numbers.
  *
- * @param url - The full URL (e.g., "https://172.0.0.18:8006" or "https://pve.example.com")
+  * @param url - The full URL (e.g., "https://proxmox-server:8006" or "https://pve.example.com")
  * @param type - The cluster type ('pve' or 'pbs') to determine default port
  * @returns Object with hostname (stripped of protocol and port) and port number
  */

PR_SUMMARY.md

@@ -12,15 +12,15 @@
 
 ## Problem
 
-Users could not add Proxmox remotes when providing URLs with port numbers (e.g., `https://172.0.0.18:8006`). The error displayed was: **"Failed to add remote"**
+Users could not add Proxmox remotes when providing URLs with port numbers (e.g., `https://proxmox-server:8006`). The error displayed was: **"Failed to add remote"**
 
 ### Root Cause
 The `RemotesPage.tsx` component incorrectly parsed URLs containing ports:
-1. User enters: `https://172.0.0.18:8006`
-2. Code strips protocol → `172.0.0.18:8006`
+1. User enters: `https://proxmox-server:8006`
+2. Code strips protocol → `proxmox-server:8006`
 3. Code uses this **with port still attached** as hostname
 4. Code **also** sends separate port parameter: `8006`
-5. Backend receives malformed: `url: "172.0.0.18:8006"` + `port: 8006`
+5. Backend receives malformed: `url: "proxmox-server:8006"` + `port: 8006`
 6. Connection fails
 
 ---
@@ -43,8 +43,8 @@ if (portMatch) {
 ```
 
 Now correctly handles:
-- ✅ Full URLs with ports: `https://172.0.0.18:8006` → hostname: `172.0.0.18`, port: `8006`
-- ✅ Hostnames only: `172.0.0.18` → hostname: `172.0.0.18`, port: `8006` (default)
+- ✅ Full URLs with ports: `https://proxmox-server:8006` → hostname: `proxmox-server`, port: `8006`
+- ✅ Hostnames only: `proxmox-server` → hostname: `proxmox-server`, port: `8006` (default)
 - ✅ Custom ports: `https://192.168.1.100:8443` → hostname: `192.168.1.100`, port: `8443`
 
 ---
@@ -83,8 +83,8 @@ Now correctly handles:
 - [x] Database corruption fixed (removed 0-byte DB)
 
 ### Required Before Merge
-- [ ] Manual test: Add remote with `https://172.0.0.18:8006`
-- [ ] Manual test: Add remote with `172.0.0.18` (should use port 8006)
+- [ ] Manual test: Add remote with `https://proxmox-server:8006`
+- [ ] Manual test: Add remote with `proxmox-server` (should use port 8006)
 - [ ] Manual test: Add PBS remote with custom port
 - [ ] Manual test: Edit existing remote and verify port changes
 - [ ] Verify remote connection succeeds

REVIEW_FIX_SUMMARY.md

@@ -2,7 +2,7 @@
 
 ## Ticket Context
 **Branch**: `fix/proxmox-remote-add-error`
-**Original Issue**: Proxmox remote URLs with ports (e.g., `https://172.0.0.18:8006`) were incorrectly parsed
+**Original Issue**: Proxmox remote URLs with ports (e.g., `https://proxmox-server:8006`) were incorrectly parsed
 
 ## Automated Review Feedback
 

TICKET-proxmox-add-remote.md

@@ -8,7 +8,7 @@ Clicking "Add Remote" in the Proxmox Remotes page always produces the error dial
 
 2. **Generic error fallback in frontend** (`src/components/Proxmox/AddRemoteForm.tsx`): The catch block uses `err instanceof Error ? err.message : 'Failed to add remote'`. Tauri errors arrive as plain strings, not `Error` objects, so the fallback branch always fires and the real error is hidden.
 
-3. **Missing protocol/port in `ProxmoxClient` URL builder** (`src-tauri/src/proxmox/client.rs`): The frontend strips the protocol via `parseRemoteUrl` before sending just the bare hostname. `get_api_url()` and `authenticate()` were constructing URLs like `"172.0.0.18/api2/json/..."` — no scheme, no port — meaning all subsequent Proxmox API calls (VMs, containers, etc.) would silently fail even after a successful add.
+3. **Missing protocol/port in `ProxmoxClient` URL builder** (`src-tauri/src/proxmox/client.rs`): The frontend strips the protocol via `parseRemoteUrl` before sending just the bare hostname. `get_api_url()` and `authenticate()` were constructing URLs like `"proxmox-server:8006/api2/json/..."` — no scheme, no port — meaning all subsequent Proxmox API calls (VMs, containers, etc.) would silently fail even after a successful add.
 
 ## Acceptance Criteria
 

opencode.jsonc

@@ -0,0 +1,3 @@
+{
+  "$schema": "https://opencode.ai/config.json"
+}

src-tauri/src/proxmox/client.rs

@@ -24,7 +24,6 @@ struct ProxmoxEnvelope<T> {
 
 /// Authentication response from Proxmox (inner `data` object).
 #[derive(Debug, Deserialize)]
-#[serde(rename_all = "PascalCase")]
 pub struct AuthResponse {
     /// Cookie value — `PVEAuthCookie=<ticket>`.
     pub ticket: String,
@@ -38,6 +37,9 @@ pub struct AuthResponse {
     /// Capability map — structure varies, only needed for display/debug.
     #[serde(default)]
     pub cap: Option<serde_json::Value>,
+    /// Cluster name
+    #[serde(default)]
+    pub clustername: Option<String>,
 }
 
 /// API token for authentication
@@ -347,13 +349,16 @@ mod tests {
     fn test_auth_response_envelope_deserialization() {
         // Validates that the `{"data": {...}}` envelope Proxmox uses is parsed
         // correctly into ProxmoxEnvelope<AuthResponse>.
+        // Note: Proxmox returns lowercase fields (ticket, username, clustername)
+        // except for CSRFPreventionToken which is PascalCase.
         let json = r#"{
             "data": {
-                "Ticket": "PVE:root@pam:12345",
-                "Username": "root@pam",
-                "Expire": 1800,
+                "ticket": "PVE:root@pam:12345",
+                "username": "root@pam",
+                "expire": 1800,
                 "CSRFPreventionToken": "abc123",
-                "Cap": null
+                "cap": null,
+                "clustername": "TFTSR"
             }
         }"#;
         let envelope: ProxmoxEnvelope<AuthResponse> =
@@ -370,8 +375,9 @@ mod tests {
         // Some Proxmox versions or API tokens may omit CSRFPreventionToken.
         let json = r#"{
             "data": {
-                "Ticket": "PVE:root@pam:99999",
-                "Username": "root@pam"
+                "ticket": "PVE:root@pam:99999",
+                "username": "root@pam",
+                "clustername": "TFTSR"
             }
         }"#;
         let envelope: ProxmoxEnvelope<AuthResponse> =
@@ -415,4 +421,159 @@ mod tests {
         assert_eq!(client.ticket.as_deref(), Some("ticket-value"));
         assert_eq!(client.csrf_token.as_deref(), Some("csrf-value"));
     }
+
+    #[tokio::test]
+    async fn test_real_proxmox_auth() {
+        let password = match std::env::var("PROXMOX_PASSWORD") {
+            Ok(p) => p,
+            Err(_) => {
+                println!("Skipping test: PROXMOX_PASSWORD env var not set");
+                return;
+            }
+        };
+
+        let mut client = ProxmoxClient::new("172.0.0.18", 8006, "root@pam");
+        let result = client.authenticate(&password).await;
+        match result {
+            Ok(ticket) => {
+                println!("✓ Authentication successful");
+                println!("  Ticket: {}", &ticket[..50]);
+                assert!(client.ticket.is_some());
+                assert!(client.csrf_token.is_some());
+            }
+            Err(e) => {
+                panic!("Authentication failed: {}", e);
+            }
+        }
+    }
+
+    #[tokio::test]
+    async fn test_real_proxmox_cluster_resources() {
+        let password = match std::env::var("PROXMOX_PASSWORD") {
+            Ok(p) => p,
+            Err(_) => {
+                println!("Skipping test: PROXMOX_PASSWORD env var not set");
+                return;
+            }
+        };
+
+        let mut client = ProxmoxClient::new("172.0.0.18", 8006, "root@pam");
+        client
+            .authenticate(&password)
+            .await
+            .expect("Authentication failed");
+
+        #[derive(serde::Deserialize, Debug)]
+        struct Resource {
+            #[serde(default)]
+            vmid: Option<u32>,
+            name: Option<String>,
+            r#type: Option<String>,
+            node: Option<String>,
+            status: Option<String>,
+        }
+
+        let result: Result<Vec<Resource>, _> = client
+            .get("cluster/resources", client.ticket.as_deref())
+            .await;
+        match result {
+            Ok(resources) => {
+                println!("✓ Cluster resources fetched successfully");
+                println!("  Found {} resources", resources.len());
+            }
+            Err(e) => {
+                panic!("Failed to get cluster resources: {}", e);
+            }
+        }
+    }
+
+    fn get_test_client() -> ProxmoxClient {
+        let host = std::env::var("PROXMOX_HOST").unwrap_or_else(|_| "proxmox-server".to_string());
+        ProxmoxClient::new(&host, 8006, "root@pam")
+    }
+
+    #[tokio::test]
+    async fn test_real_proxmox_nodes() {
+        let password = match std::env::var("PROXMOX_PASSWORD") {
+            Ok(p) => p,
+            Err(_) => {
+                println!("Skipping test: PROXMOX_PASSWORD env var not set");
+                return;
+            }
+        };
+
+        let mut client = ProxmoxClient::new("proxmox-server", 8006, "root@pam");
+        client
+            .authenticate(&password)
+            .await
+            .expect("Authentication failed");
+
+        #[derive(serde::Deserialize, Debug)]
+        struct Node {
+            node: String,
+            status: String,
+            #[serde(default)]
+            level: String,
+            #[serde(default)]
+            cpu: f64,
+            #[serde(default)]
+            uptime: u64,
+        }
+
+        let result: Result<Vec<Node>, _> = client.get("nodes", client.ticket.as_deref()).await;
+        match result {
+            Ok(nodes) => {
+                println!("✓ Nodes fetched successfully");
+                for node in &nodes {
+                    println!("  Node: {} - Status: {}", node.node, node.status);
+                }
+            }
+            Err(e) => {
+                panic!("Failed to get nodes: {}", e);
+            }
+        }
+    }
+
+    #[tokio::test]
+    async fn test_real_proxmox_vms() {
+        let password = match std::env::var("PROXMOX_PASSWORD") {
+            Ok(p) => p,
+            Err(_) => {
+                println!("Skipping test: PROXMOX_PASSWORD env var not set");
+                return;
+            }
+        };
+
+        let mut client = ProxmoxClient::new("proxmox-server", 8006, "root@pam");
+        client
+            .authenticate(&password)
+            .await
+            .expect("Authentication failed");
+
+        #[derive(serde::Deserialize, Debug)]
+        struct Resource {
+            #[serde(default)]
+            vmid: Option<u32>,
+            name: Option<String>,
+            r#type: Option<String>,
+            status: Option<String>,
+        }
+
+        let result: Result<Vec<Resource>, _> = client
+            .get("cluster/resources", client.ticket.as_deref())
+            .await;
+        match result {
+            Ok(resources) => {
+                let vms: Vec<_> = resources
+                    .into_iter()
+                    .filter(|r| r.r#type.as_deref() == Some("qemu"))
+                    .collect();
+                println!("✓ VMs fetched successfully");
+                println!("  Found {} VMs", vms.len());
+            }
+            Err(e) => {
+                panic!("Failed to get VMs: {}", e);
+            }
+        }
+    }
 }

src/pages/Proxmox/RemotesPage.tsx

@@ -55,7 +55,7 @@ export function ProxmoxRemotesPage() {
    * Helper function to parse a Proxmox URL and extract hostname and port.
    * Handles URLs with or without explicit port numbers.
    *
-   * @param url - The full URL (e.g., "https://172.0.0.18:8006" or "https://pve.example.com")
+    * @param url - The full URL (e.g., "https://proxmox-server:8006" or "https://pve.example.com")
    * @param type - The cluster type ('pve' or 'pbs') to determine default port
    * @returns Object with hostname (stripped of protocol and port) and port number
    */