- Added YAML frontmatter to all 8 polish-* skills and sweep skill so Claude can auto-invoke them - New bitcoin-conventions skill with PROUX UX methodology, sats display, address validation, Tor preferences, Lightning patterns - Path-specific rules for containers (security hardening) and frontend (Vue/glassmorphism conventions) - Gitea Actions: nightly security review and weekly dependency audit Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
157 lines
4.4 KiB
Markdown
157 lines
4.4 KiB
Markdown
---
|
|
name: polish-backend
|
|
description: Fix Rust backend quality issues in Archipelago. Eliminates panics/unwraps, adds timeouts, implements connection pooling, fixes clippy warnings. Use when user says "polish backend", "fix unwraps", "backend quality", or "eliminate panics".
|
|
---
|
|
|
|
# Skill: Polish Backend Quality
|
|
|
|
Fix Rust backend quality issues: eliminate panics, add timeouts, implement connection pooling, fix clippy warnings. The backend must never crash in production.
|
|
|
|
## Priority 1: Eliminate Panics
|
|
|
|
### Find all unwrap/expect in production code
|
|
```bash
|
|
ssh archipelago@192.168.1.228 "cd ~/archy && grep -rn 'unwrap()\|\.expect(' core/archipelago/src/ core/container/src/ core/security/src/ core/performance/src/ --include='*.rs' | grep -v test | grep -v '#\[test\]' | grep -v '_test.rs'"
|
|
```
|
|
|
|
### Fix patterns:
|
|
|
|
**Response builder unwraps** (handler.rs):
|
|
```rust
|
|
// BAD
|
|
Response::builder().body(body).unwrap()
|
|
|
|
// GOOD
|
|
Response::builder().body(body).map_err(|e| {
|
|
tracing::error!("Failed to build response: {}", e);
|
|
// Return a minimal 500 response
|
|
})?
|
|
```
|
|
|
|
**Socket address parsing** (main.rs):
|
|
```rust
|
|
// BAD
|
|
addr.parse().expect("Invalid bind address")
|
|
|
|
// GOOD
|
|
addr.parse().context("Invalid bind address")?
|
|
```
|
|
|
|
**TOTP secret creation** (totp.rs):
|
|
```rust
|
|
// BAD
|
|
TOTP::new(...).unwrap()
|
|
|
|
// GOOD
|
|
TOTP::new(...).map_err(|e| anyhow::anyhow!("Failed to create TOTP: {}", e))?
|
|
```
|
|
|
|
**Cosign URL parsing** (image_verifier.rs):
|
|
```rust
|
|
// BAD
|
|
sig_url.strip_prefix("cosign://").unwrap()
|
|
|
|
// GOOD
|
|
sig_url.strip_prefix("cosign://")
|
|
.ok_or_else(|| anyhow::anyhow!("Invalid cosign URL format: {}", sig_url))?
|
|
```
|
|
|
|
## Priority 2: Add Timeouts
|
|
|
|
Every external call must have an explicit timeout:
|
|
|
|
```rust
|
|
// Container operations
|
|
tokio::time::timeout(Duration::from_secs(30), podman_operation()).await
|
|
.context("Container operation timed out after 30s")??;
|
|
|
|
// HTTP calls (Bitcoin RPC, LND proxy)
|
|
let client = reqwest::Client::builder()
|
|
.timeout(Duration::from_secs(10))
|
|
.build()?;
|
|
|
|
// Nostr operations
|
|
tokio::time::timeout(Duration::from_secs(15), nostr_publish()).await
|
|
.context("Nostr publish timed out")?;
|
|
```
|
|
|
|
## Priority 3: Connection Pooling
|
|
|
|
Store a reusable `reqwest::Client` in `RpcHandler`:
|
|
```rust
|
|
pub struct RpcHandler {
|
|
// ... existing fields
|
|
http_client: reqwest::Client,
|
|
}
|
|
|
|
impl RpcHandler {
|
|
pub fn new(...) -> Self {
|
|
let http_client = reqwest::Client::builder()
|
|
.timeout(Duration::from_secs(10))
|
|
.pool_max_idle_per_host(5)
|
|
.build()
|
|
.expect("Failed to create HTTP client");
|
|
// ...
|
|
}
|
|
}
|
|
```
|
|
|
|
Use `self.http_client` everywhere instead of creating new clients per request.
|
|
|
|
## Priority 4: Fix Clippy Warnings
|
|
|
|
Run on dev server:
|
|
```bash
|
|
ssh archipelago@192.168.1.228 "cd ~/archy && cargo clippy --all-targets --all-features 2>&1"
|
|
```
|
|
|
|
Known warnings to fix:
|
|
- `should_implement_trait`: Implement `FromStr` for `AppManifest`
|
|
- `get_first` → `.first()`
|
|
- `assign_op_pattern` → use `+=`
|
|
- `wildcard_in_or_patterns` → remove redundant `_`
|
|
- `redundant_field_names` → shorthand
|
|
- `very_complex_type` → type alias
|
|
- `if_else_collapse` → simplify
|
|
|
|
## Priority 5: Replace println with tracing
|
|
|
|
```bash
|
|
ssh archipelago@192.168.1.228 "cd ~/archy && grep -rn 'println!\|eprintln!' core/ --include='*.rs' | grep -v test | grep -v target/"
|
|
```
|
|
|
|
Replace:
|
|
- `println!("...")` → `tracing::info!("...")`
|
|
- `eprintln!("...")` → `tracing::warn!("...")`
|
|
|
|
## Priority 6: Remove Dead Code
|
|
|
|
- Remove `#[allow(dead_code)]` annotations, verify if types are actually used
|
|
- Remove unused fields (e.g., `identity_dir` in NodeIdentity)
|
|
- Remove unused methods (e.g., `verify()`, `did_key()` in NodeIdentity)
|
|
|
|
## Verification
|
|
|
|
```bash
|
|
ssh archipelago@192.168.1.228 "cd ~/archy && cargo clippy --all-targets --all-features 2>&1 | grep -c 'warning'"
|
|
# Should be 0
|
|
|
|
ssh archipelago@192.168.1.228 "cd ~/archy && grep -rn 'unwrap()\|\.expect(' core/archipelago/src/ --include='*.rs' | grep -v test | grep -v '_test.rs' | wc -l"
|
|
# Should be 0 (or near-zero with justified exceptions)
|
|
|
|
ssh archipelago@192.168.1.228 "cd ~/archy && grep -rn 'println!\|eprintln!' core/ --include='*.rs' | grep -v test | grep -v target/ | wc -l"
|
|
# Should be 0
|
|
```
|
|
|
|
## Build & Deploy
|
|
|
|
All Rust changes MUST be built on the dev server, never macOS:
|
|
```bash
|
|
./scripts/deploy-to-target.sh --live
|
|
```
|
|
|
|
After deploy, verify:
|
|
```bash
|
|
ssh -i ~/.ssh/archipelago-deploy archipelago@192.168.1.228 "systemctl status archipelago && curl -s http://localhost:5678/health"
|
|
```
|