chore: improve CLAUDE.md with architecture docs and add Claude automations

.claude/agents/clippy-reviewer.md

@@ -0,0 +1,19 @@
+---
+name: clippy-reviewer
+description: Runs cargo clippy with all warnings and reviews findings after implementation work
+---
+
+# Clippy Reviewer
+
+Run `cargo clippy -- -W clippy::all` and analyze the output.
+
+## Process
+
+1. Run `cargo clippy -- -W clippy::all 2>&1`
+2. Parse warnings and errors
+3. For each finding:
+   - Explain what the lint catches and why it matters
+   - Suggest the idiomatic fix
+   - Note if it's a false positive given the project context
+4. If there are auto-fixable lints, suggest running `cargo clippy --fix`
+5. Report a summary: total warnings, grouped by category

.claude/agents/security-reviewer.md

@@ -0,0 +1,23 @@
+---
+name: security-reviewer
+description: Reviews code changes in api/, config/credentials.rs, and auth-related code for security issues
+---
+
+# Security Reviewer
+
+You are a security-focused code reviewer for bbctl, a CLI/TUI tool that manages infrastructure on VyOS and Proxmox servers.
+
+## Focus Areas
+
+- **Credential handling**: Ensure passwords, API keys, and SSH keys are never logged, hardcoded, or exposed in error messages
+- **TLS configuration**: Flag any use of `danger_accept_invalid_certs` that isn't behind a configuration flag or warning
+- **Command injection**: Check `tokio::process::Command` usage for unsanitized input in SSH commands
+- **TOML deserialization**: Verify config parsing handles malicious input gracefully
+- **Error messages**: Ensure error output doesn't leak sensitive information (credentials, internal paths)
+
+## Review Process
+
+1. Read the changed files (focus on `src/api/`, `src/config/credentials.rs`, `src/services/provider.rs`)
+2. Check for the focus areas above
+3. Report findings with severity (critical/warning/info) and file:line references
+4. If no issues found, confirm the changes look secure

.claude/skills/check.md

@@ -0,0 +1,12 @@
+---
+name: check
+description: Run cargo check, clippy, and fmt --check to validate the bbctl codebase
+---
+
+Run the following commands sequentially, reporting results after each. Stop on first failure:
+
+1. `cargo fmt --check` — verify formatting
+2. `cargo clippy -- -W clippy::all` — lint with all warnings enabled
+3. `cargo check` — verify compilation
+
+Report a summary of any issues found. If all pass, confirm the codebase is clean.

.claude/skills/test-scaffold.md

@@ -0,0 +1,29 @@
+---
+name: test-scaffold
+description: Generate test module for a Rust source file using project conventions (assert_cmd, mockito, tempfile)
+disable-model-invocation: true
+---
+
+# Test Scaffold Generator
+
+Generate idiomatic Rust tests for a given source file in bbctl.
+
+## Arguments
+
+- `$ARGUMENTS` — path to the source file to generate tests for (e.g., `src/config/mod.rs`)
+
+## Instructions
+
+1. Read the target source file
+2. Identify all public functions, structs, and trait implementations
+3. Generate a `#[cfg(test)]` module at the bottom of the file with tests for each public item
+4. Use the project's dev-dependencies:
+   - `tempfile` for tests that need filesystem operations (config files)
+   - `mockito` for tests that make HTTP requests (API clients)
+   - `assert_cmd` for CLI integration tests (main.rs)
+5. Follow these patterns:
+   - Test function naming: `test_<function_name>_<scenario>`
+   - Use `#[test]` for sync tests, `#[tokio::test]` for async tests
+   - Include both happy path and error cases
+   - For config tests, use `tempfile::tempdir()` instead of writing to `~/.bbctl/`
+6. Show the generated test module and ask for confirmation before writing

.mcp.json

@@ -0,0 +1,8 @@
+{
+  "mcpServers": {
+    "context7": {
+      "command": "npx",
+      "args": ["-y", "@upstash/context7-mcp@latest"]
+    }
+  }
+}

CLAUDE.md

@@ -1,79 +1,130 @@
-# bbctl Development Guidelines
+# CLAUDE.md
 
-## Build & Run Commands
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
 
-```bash
-# Build
-cargo build
+## Build & Test Commands
 
-# Run
-cargo run
+```bash
+cargo build                          # Dev build
+cargo build --release                # Release build
+cargo run                            # Launch TUI (no args)
+cargo run -- instances list          # CLI mode with subcommand
+cargo run -- test-vyos --host HOST --port 60022 --username vyos --api-key KEY
+cargo test                           # All tests
+cargo test test_name -- --nocapture  # Single test with output
+cargo fmt                            # Format
+cargo clippy                         # Lint
+```
 
-# Run with specific command
-cargo run -- [command] [subcommand] [args]
+## What This Project Is
 
-# Build optimized release version
-cargo build --release
+bbctl is a CLI/TUI tool for provisioning multi-tenant infrastructure on bare metal servers running VyOS v1.5 or Proxmox. It provides both a Clap-based CLI and an interactive Ratatui terminal dashboard.
 
-# Run specific test
-cargo test test_name -- --nocapture
+## Architecture
 
-# Format code
-cargo fmt
+Five-layer design with strict dependency direction (upper layers depend on lower):
 
-# Lint code
-cargo clippy
+```
+CLI (main.rs) / TUI (app.rs, ui.rs, handler.rs, event.rs, tui.rs)
+                            |
+              Services (services/) — orchestration, client factory
+                            |
+              Models (models/) — domain entities with business logic
+                            |
+              Config (config/) — TOML persistence (~/.bbctl/)
+                            |
+              API (api/) — provider-specific HTTP/SSH clients
 ```
 
-## CLI Examples
+### Entry Point (main.rs)
 
-```bash
-# List instances
-cargo run -- instances list
+`#[tokio::main]` dispatches two paths:
+- **CLI mode**: `env::args().len() > 1` → `cli_handler()` processes Clap subcommands synchronously
+- **TUI mode**: No args → `run_tui()` launches the async Ratatui event loop
+- **Special case**: `test-vyos` subcommand is matched before `cli_handler()` because it needs the async runtime for SSH/HTTP calls
 
-# Create instance
-cargo run -- instances create my-instance --provider vyos --region nyc --cpu 2 --memory 4 --disk 80
+Clap subcommands: `init`, `deploy`, `instances {list|create|delete|start|stop|show}`, `volumes {list|create|delete|attach|detach|show}`, `networks {list|create|delete|connect|disconnect|show}`, `test-vyos`
 
-# Create volume
-cargo run -- volumes create my-volume --size 10 --region nyc
+### Provider System (api/)
 
-# Create network
-cargo run -- networks create my-network --cidr 192.168.1.0/24
+`Provider` trait in `api/mod.rs` defines the interface (uses `anyhow::Result`):
+```rust
+pub trait Provider {
+    fn connect(&self) -> Result<()>;
+    fn check_connection(&self) -> Result<bool>;
+    fn name(&self) -> &str;
+}
 ```
 
-## Code Style Guidelines
+Two implementations:
+- **VyOSClient** (`api/vyos.rs`) — SSH via `tokio::process` + HTTP API with `reqwest`. Key methods: `execute_ssh_command()`, `api_call()`, `get_config()`, `set_config()`, `commit()`, `save()`, `get_system_info()`
+- **ProxmoxClient** (`api/proxmox.rs`) — HTTP API with ticket/CSRF auth or API token. Key methods: `login()`, `api_call()`. Auth enum: `ProxmoxAuth::UserPass` or `ProxmoxAuth::ApiToken`
+
+Both use `reqwest::Client` with `danger_accept_invalid_certs` for self-signed certs.
+
+### Services Layer (services/)
+
+`ProviderService` is the factory that bridges config → API clients:
+- Loads `Providers` + `Credentials` from TOML
+- `get_vyos_client()` / `get_proxmox_client()` instantiate typed clients from stored config + credentials
+- `add_vyos_provider()` / `add_proxmox_provider_with_token()` register new providers
+
+`InstanceService`, `VolumeService`, `NetworkService` each hold in-memory storage (`HashMap<Uuid, T>`) plus a `ProviderService` reference.
+
+### Config System (config/)
+
+Config dir: `~/.bbctl/` (constant `APP_DIR_NAME`). Three TOML files:
+- **settings.toml** — global settings (default provider, region, log level)
+- **providers.toml** — provider configs + regions (HashMap-based, keyed by name)
+- **credentials.toml** — auth credentials, intentionally separated from provider configs
+
+Helper functions: `get_config_dir()`, `get_config_file()`, `read_config_file()`, `write_config_file()`, `config_file_exists()`, `delete_config_file()`, `init_config()` (creates defaults on first run).
 
-- **Formatting**: Use `cargo fmt` to format code according to Rust standard style
+### TUI Architecture
 
-- **Linting**: Run `cargo clippy` for static analysis
+Event loop: `App::new()` → `EventHandler` (spawns tokio task reading `crossterm::EventStream` with 250ms tick) → main loop calls `tui.draw()` then matches on `Event::{Tick, Key, Mouse, Resize}`.
 
-- **Naming**:
+`AppMode` enum (`Home | Instances | Volumes | Networks | Settings | Help`) drives which `render_*()` function runs. `selected_index` tracks list navigation. Keys: `1-5` jump modes, `j/k` navigate, `q/ESC` back/quit, `?` help.
 
-- Use snake_case for variables, functions, and modules
+Note: `app.rs` contains both TUI state (`App`, `AppMode`) and simple display structs (`Instance`, `Volume`, `Network`) used for hardcoded demo data. The richer domain types live in `models/`.
 
-- Use PascalCase for structs, enums, and traits
+### Models (models/)
 
-- **Error Handling**: Use `AppResult<T>` for functions that can fail
+Domain types with business logic methods (separate from the simpler display structs in `app.rs`):
+- **Instance** (`models/instance.rs`) — UUID id, `InstanceStatus` enum, `InstanceSize {cpu, memory_gb, disk_gb}`, `Vec<InstanceNetwork>`, tags HashMap
+- **Volume** (`models/volume.rs`) — `VolumeStatus`/`VolumeType` enums, `attach()`/`detach()`/`extend()` methods with validation
+- **Network** (`models/network.rs`) — `NetworkType` enum, `HashSet<Uuid>` for connected instances, `Vec<IpAllocation>` with `allocate_ip()`/`release_ip()`
+- **Provider** (`models/provider.rs`) — `ProviderType` enum used by other models
 
-- **State Management**: Follow the App/AppMode pattern for managing application state
+## Error Handling
 
-- **UI Components**: Use Ratatui components (List, Table, Paragraph) with consistent styling
+Two error types coexist:
+- `AppResult<T> = Result<T, Box<dyn error::Error>>` in `app.rs` — used by TUI and main
+- `anyhow::Result<T>` — used by config, API, and services layers (with `anyhow::Context` for wrapping)
 
-- **Provider APIs**: VyOS and Proxmox providers should implement common traits
+## Key Dependencies
 
-- **Imports**: Group imports by crate, with std first, then external, then internal
-- **Document**: Use three slashes (`///`) for public API documentation
-- **Async**: Use tokio runtime with futures for async operations
+- **ratatui** + **crossterm** (event-stream) — TUI framework
+- **clap** (derive) — CLI parsing
+- **tokio** (full) — async runtime
+- **reqwest** (json, rustls-tls) — HTTP client
+- **serde** + **toml** — config serialization
+- **uuid** (v4, serde) — resource identifiers
+- **anyhow** — error handling in lower layers
+- **chrono** (serde) — timestamps on domain models
 
-## Project Structure
+## Code Style
 
-- **src/app.rs**: Core application state and data models
-- **src/event.rs**: Event handling for TUI (keyboard, mouse, resize)
-- **src/handler.rs**: Keyboard event processing
-- **src/tui.rs**: Terminal setup and management
-- **src/ui.rs**: UI rendering and layout components
-- **src/main.rs**: CLI command processing using Clap
+- **Formatting**: `cargo fmt` (standard rustfmt)
+- **Linting**: `cargo clippy`
+- **Naming**: snake_case (vars/fns/modules), PascalCase (structs/enums/traits)
+- **Imports**: Group by crate — std first, then external, then internal
+- **Docs**: `///` for public API documentation
 
-The app is organized following a typical TUI pattern with app state, event handling, and UI rendering modules. Follow existing patterns when adding new functionality.
+## Current State
 
-Future work includes integrating with actual VyOS and Proxmox APIs and adding E2E encryption for public cloud integration.
+- CLI handler uses hardcoded println output (not yet connected to real providers)
+- TUI displays hardcoded demo data from `App::new()`
+- Provider trait only covers connection; no CRUD traits for resources yet
+- Services use in-memory `HashMap` storage, no persistence layer
+- `models/` has rich domain types but they're not yet wired into `app.rs` display structs

src/api/proxmox.rs

@@ -1,8 +1,8 @@
 use anyhow::{Result, Context, anyhow};
-use reqwest::{Client, StatusCode};
+use reqwest::Client;
 use serde::{Deserialize, Serialize};
 use std::time::Duration;
-use log::{debug, error, info};
+use log::{debug, info};
 
 use crate::api::Provider;
 

src/api/vyos.rs

@@ -1,5 +1,5 @@
 use anyhow::{Result, Context, anyhow};
-use reqwest::{Client, StatusCode};
+use reqwest::Client;
 use serde::{Deserialize, Serialize};
 use std::process::Command;
 use tokio::process::Command as AsyncCommand;

src/config/credentials.rs

@@ -1,10 +1,9 @@
 use serde::{Deserialize, Serialize};
 use anyhow::{Result, Context, anyhow};
 use std::collections::HashMap;
-use log::{debug, info, error};
+use log::{debug, info};
 
 use crate::config::{read_config_file, write_config_file, CREDENTIALS_FILE};
-use crate::models::provider::ProviderType;
 
 /// VyOS credentials
 #[derive(Debug, Clone, Serialize, Deserialize)]

src/config/mod.rs

@@ -2,9 +2,9 @@ pub mod provider;
 pub mod settings;
 pub mod credentials;
 
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 use anyhow::{Result, Context, anyhow};
-use log::{debug, info, error};
+use log::{debug, info};
 use std::fs;
 use dirs::home_dir;
 

src/config/provider.rs

@@ -1,7 +1,7 @@
 use serde::{Deserialize, Serialize};
 use anyhow::{Result, Context, anyhow};
 use std::collections::HashMap;
-use log::{debug, info, error};
+use log::{debug, info};
 
 use crate::config::{read_config_file, write_config_file, PROVIDERS_FILE};
 use crate::models::provider::{ProviderType, ProviderConfig, Region};
@@ -102,7 +102,7 @@ impl Providers {
 
         // Ensure the provider exists
         let provider_name = region.provider.to_string();
-        if !self.providers.iter().any(|(name, p)| p.provider_type == region.provider) {
+        if !self.providers.iter().any(|(_name, p)| p.provider_type == region.provider) {
             return Err(anyhow!("Provider '{}' does not exist", provider_name));
         }
 

src/config/settings.rs

@@ -1,7 +1,6 @@
 use serde::{Deserialize, Serialize};
 use anyhow::{Result, Context, anyhow};
-use std::fs;
-use log::{debug, info, error};
+use log::{debug, info};
 
 use crate::config::{read_config_file, write_config_file, SETTINGS_FILE};
 

src/main.rs

@@ -290,7 +290,7 @@ fn cli_handler(cli: Cli) -> AppResult<()> {
                 }
             }
         }
-        Some(Commands::TestVyOS { host, port, username, .. }) => {
+        Some(Commands::TestVyOS { .. }) => {
             // This would block, so we need to call it outside the CLI handler
             // Will be implemented in main()
             return Err("Use tokio runtime to test VyOS connectivity".into());
@@ -378,7 +378,7 @@ async fn main() -> AppResult<()> {
                         println!("\n✅ SSH connection successful!");
 
                         // If API key is provided, also test the API
-                        if let Some(api_key) = &api_key {
+                        if api_key.is_some() {
                             println!("\nTesting VyOS HTTP API...");
 
                             let mut client_mut = client;